Upsell items truncated by one product in product class/taxonomy item

That's quite odd... and I can't say I've ever seen that behavior.. I'll look into it some more.

Just so we're clear, it *only* does this when you're only doing the "Override" for taxonomy-related products? And it only does it for certain vocabs? Seems pretty strange... have you run Cron lately? Perhaps the array of taxonomy-related nodes needs to be rebuilt?

I think this might be a taxonomy term issue. My guess is that perhaps it's going by Term, and not Vocabulary. Can you explain to me a little more how your Catalog vocabulary is setup? In my test setup I have a "Products" vocab, with two terms: "Doodads" and "Widgets."

When I set the system to "Add" and I had 6 or 7 products belonging to the "Widgets" term, I got 5 products, as I would expect. The only way I could get this to NOT happen was when I didn't have enough products belonging to that term id. One way you can check is, in the uc_upsell_core.inc file, go to the function called uc_upsell_nids_by_term. Then before the "return $nids" line, add this:

drupal_set_message('got nids from db: '.print_r($nids, true));

Then refresh a node page. That will show you all of the nodes it got back from the database call that basically is saying "what's the node we're looking at, and what term does it belong to?" From there it strips out the current node, because you don't want the product you're looking at to show up in the "related" block. In my case I was able to see that when my current node got stripped out, I was missing 1 product - but only because I didn't have enough associated with the "Widgets" term to begin with.

Give that a shot - add more products to one specific product's term. If that seems to be the case, maybe I'm being too granular with my taxonomy associations, and I need to go a level higher so that it considers parent terms as well? So for instance if "Widgets" had a sub-term called "Cubits" I wouldn't want to look at just the Cubits term, necessarily, but still stay on the parent term, which is "Widgets." Does this make sense?

Right now it still seems to work as it should, assuming you have enough products that belong to the term in question.

Are you still seeing this problem?

That's okay, it was only a very minor change between 1.19 and 1.20, but I want to make 1.20 the final release (frozen code) so that I can focus putting new features into 2.0.

Thanks :)

I just upgraded from 1.15 to 1.20 and I'm having this issue as well. Off by one error.

It happens in uc_upsell_filter_types

IN (605,186,542,193)
product = 605, status = 0, 
product = 186, status = 1, 
product = 542, status = 1, 
product = 193, status = 1, 
OUT (186,542,193)

The function is correctly filtering out the un-published node...but it's not filling it up with a random or published node (of which there are)

I tracked the issue down to this section of code, which is limiting the results too much.

  // First, if we are including related-by-taxonomy nodes, and an array exists, only include as many as needed
  if (count($related_by_term) > 0 && count($related) < $max) {
    $needed = $max - count($related); // How many do we need?

    if ($needed > 0) {
      // Filter out the nodes already in our list
      $related_by_term = array_diff(array_values($related_by_term), array_values($related));
      // We're slicing too soon. 
      //$related_by_term = array_slice($related_by_term, 0, $needed);
      $related = array_unique(array_merge((array)$related, (array)$related_by_term));
    }
  }

If you comment out the array_slice line everything works fine. I have done so in my code. Another thing I noticed because of this slice, was the lack of randomization due to the fact that it's only randomizing off my "slice of 4" instead of out of all the related nodes.

This fix alone will not solve the problem, as we could have 1 published node and the rest are unpublished, we'll still run into the previous issue I mentioned. We should be only pulling in published nodes into the "needed" array.

Thanks a bunch for this module, it's one of my faves for Ubercart. looking forward to the 2.x branch.

I'm seeing this as well.

Crazy - thanks for the updates and reopening the bug. I'll look into it this week, along with the other Issue I have on my radar for Upsell, and I'll release a new version soon as I can.

Here's a patch against the current 1.x dev, which I think is the same as the 1.20 (Haven't made any major commits since then). If it doesn't work let me know and I will re-roll it against 1.20.

What it does is move the "add randoms" functionality to after the node type filter. This is how it was originally meant to behave - only add randoms (which will not honor some filtering-by-type rules) only once we've filtered out too many. Make sure the "add random products" checkbox is also checked in the "Cart Pane" settings at admin/store/settings/upsell.

It seems to work in my testing but I'd appreciate some feedback :)

Whoops, latest patch had an incorrect array_merge($related) - should be array_merge($ary). Here's an updated patch.

If there's one thing I'm good at, it's breaking things :D

Thanks for the fix TP. This is a great module. Thanks for your hard work.

j0rd, thank YOU for your feedback and breaking stuff. That's the only way I can make anything better :)

Sounds good. Keep me posted!

That seems to have solved the problem for me. Thanks!

Sweet :)

Awesome news! I'll commit this in the next release :) Thanks for your help!

Committed in CVS.