I'm not quite sure if this qualifies as a bug or not, but cart items aren't loaded in what seems the most logical method; which seems it would be that the first added item should be first in the list..

A simple ORDER BY addition to the SQL query that pulls the contents from the database in the uc_cart_get_contents() function fixes this.

I've supplied two patches here, one orders items by cart_item_id which should always be the order items were added to the cart, the other by changed, which should usually be the order items were added to the cart, unless the visitor changed quantities, which would place the most recently changed item at the bottom of the list.

These patches simply alter the order of the results from the SQL query, they don't supply any additional settings. If other people are interested in this I can whip up a patch that will add a setting for selecting the method to use for the order to pull items from the db to build the cart. Options I can think of at the moment would be:

  • Products added first at the top of the list, with each newly added product following in the order added
  • Reverse of above, Newest product first, oldest last
  • Same as above, but based on the timestamp in the changed column - newest first or oldest first
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

stephthegeek’s picture

I can't comment on the technical side of things, but this makes a whole lot of sense. It's something I never really paid attention to but kind of a wtf for most stores now that I think about it. First added at the top seems to be most obvious to me.

rszrama’s picture

Yeah, this has been quite a pain indeed, I'm just not sure what implications this might have for other modules that expect the current behavior. I know it sounds odd, because this is obviously an improvement, but every little improvement seems to have unfortunate side effects. Lemme sit on it and maybe consider it for post major release.

willvincent’s picture

I've whipped up a patch that lets you turn this functionality on and decide what order to display items in the cart, or off and leave it as is..

The reason I bring this up is that it's important to be sure items are displayed first product added first for things to make sense with my hotel module. ;) Worst case scenario though I can distribute a patch along with the module for a while if need be.

willvincent’s picture

FileSize
1.95 KB

Patch mentioned in previous comment. Adds the following options to cart settings screen in admin...

Cart contents sort order preference

  • Unsorted
  • Oldest added items first
  • Newest added items first
  • Oldest added or changed items first
  • Newest added or changed items first

Defaults to Unsorted to maintain current functionality.

Island Usurper’s picture

Status: Needs review » Needs work

I think db_query() is filtering out that second %s because it's not in quotes. Or something. I had to add $cart_order_preference directly to the query to get it to work.

For what it's worth, whenever you update the quantities in the cart, all of the items get the same changed value, just because the form was submitted. So, it's better to have just ORDER BY cart_item_id, since that column is always unique per item.

I'm not really happy with the fact that you have to change the order in the query with a variable. My vote goes to just hard-code ORDER BY cart_item_id ASC and not have a setting for it like the original patch. It seems like a reasonable enough default.

willvincent’s picture

That's odd that the second %s wouldn't work for you, as it should just be inserting the rest of the SQL statement, (and it worked for me under both linux and windows).

I agree that a hard coded value is probably better, seems like it's a more secure solution.

willvincent’s picture

Bumping this up to get it back onto Ryan's radar. ;)

TR’s picture

Version: 6.x-2.0-rc7 » 6.x-2.x-dev

Putting this one on my radar...

gooddesignusa’s picture

subscribe

etaroza’s picture

I think this could be related: Cart items have the same "changed" date

pebosi’s picture

FileSize
2.14 KB

Created a changed patch to solve this.

neilnz’s picture

Priority: Normal » Major
Status: Needs work » Needs review

This bug causes much bigger problems that appear when the database does not consistently return the cart items in the same order. It seems the expected default behaviour from MySQL is to return them in the order they were last written (which means that if you update the quantity of a product, that product drops to the last line in the cart), but it gets worse if your site uses master/slave replication with query splitting, and the two servers don't return the cart items in the same order without an ORDER BY.

Because the cart form uses indexes rather than cart item IDs to update the products, if the order of cart items coming from back the database changes in between rendering the list and submitting a quantity update, it actually updates the quantity of the wrong products (the ones that were in that list position previously).

I highly recommend just committing the original patch cart_item_id_order.patch from the original issue, rather than allowing users to select the order they want.

This issue is also present in the D7 version.

TR’s picture

@neilnz: Did you test the original patch, and does it work? There hasn't been any review of the patch yet.

neilnz’s picture

Status: Needs review » Reviewed & tested by the community

@TR, yes I've applied it to fix the problems I was encountering. It works fine, although if I wrote it I might use "ORDER BY c.cart_item_id ASC" rather than just "ORDER BY c.cart_item_id", just to be explicit.

TR’s picture

OK. Can you create one patch rolled against the current -dev, with the ASC, rather than the two patches in the initial post. If not, that's OK, but it does make it easier for the maintainers to evaluate and commit these things.

neilnz’s picture

Sure, here you go.

TR’s picture

Status: Reviewed & tested by the community » Fixed

Thanks. Committed to both 6.x-2.x and 7.x-3.x.

Status: Fixed » Closed (fixed)

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