Creating a new issue related to #1120776: order number increments on reload checkout page, #1057488: Order object lifetime and save issues in Ajax checkout, #273574: Reduce HTTP_REFERER checks and #1281514: uc_quote creates line-item for order_id==0 which is never deleted.

There has been discussion of moving the creation of a new row in the 'orders' table into the initial checkout form builder instead of the validate step. The main advantage of this is that modules can only attach data (line items, comments, etc.) once an order has been saved to the database (and thus assigned an order #). The main concern is that this would cause the proliferation of abandoned 'in_checkout' orders.

The attached patch creates the order on form-build, and tries to minimize abandoned orders by allowing them to be recycled within a session.

Comments

wodenx’s picture

StatusFileSize
new7.96 KB

Here's the patch. Note this must be applied on top of the one at #273574: Reduce HTTP_REFERER checks.

longwave’s picture

The worry I have about recycling order numbers is that I think they should be roughly sequential, and deviating from this too much could confuse store owners.

On a busy store, say someone visits checkout and is given an initial order number, but does not complete checkout. Then they return to the site periodically (so they don't lose their session) and finally decide to complete their purchase six months later. With this patch they will still have the initial order number, which could be thousands behind the "normal" order sequence.

longwave’s picture

This will also never make order numbers entirely sequential as some customers will never return to recycle their order number. To achieve that we need a different mechanism, whereby we assign an "invoice number" separate from the order number when the customer completes checkout.

I am not sure I get the point of reducing wasted order numbers; they are not a scarce resource.

wodenx’s picture

I guess it's a trade-off between having many wasted orders and having non-chronological order numbers. It's true that order numbers aren't a scarce resource, but it's annoying to have a lot of meaningless 'in_checkout' orders hanging around. Nevertheless, I've removed the recycling code from the patch. I've also broken it down into four distinct changes. These could be separate issues, but I'm attaching them all here since they have to be applied in order.

1-Create order on initial checkout.

2-Make the order timeout configurable: As I say, to me 20 mins seems too long, so I imagine every store owner will have a different idea about it.

3-Prevent the cart order from being reset every time user visits the cart page: Not only reduces wasted order numbers, but allows last minute impulse buys without resetting order details. But will it have other ramifications?

4-remove the serialized cart contents from the form: What was the purpose of this?

Re: sequential order numbers - I agree, the "invoice number" idea is the only way to guarantee that - that's what I was getting at in #1120776: order number increments on reload checkout page #9

longwave’s picture

1. This will actually increase wasted order numbers on its own, as right now we don't create an order number until someone submits the checkout page. But this makes things easier for contrib modules as they are guaranteed an order number to work with.

2. If you think 20 minutes is too long, what do you consider reasonable? 10 minutes? I'm against adding options for the sake of it, that many store owners will likely not care about or want to change; where possible I'm trying to remove settings that are not strictly needed (credit card debug mode, option to disable payment and order logging, etc). I think we should also fix an upper limit here as an increased limit has security ramifications (in the event that someone enters payment details but doesn't complete the order, and someone else then uses the same browser to complete the order)

3. I think this could be implemented but as we have both figured this may have side effects elsewhere, and these need to be investigated first. I think we can safely remove the one in uc_cart_view() at least, as I can't see any side effects there.

4. The serialized cart contents are leftovers from D6 where they were used for Ajax line item updates, I think this can be safely removed.

Another thing we could do is cancel or delete in_checkout orders that are being deliberately abandoned due to the timeout. This could also be done on cron so any in_checkout orders abandoned due to the customer closing their browser can be cancelled or deleted. Adding a Rules event for this could be useful, allowing store owners to leave them in_checkout, change the status, delete them entirely, and/or add an order comment.

wodenx’s picture

1. I know - that's why i was trying to reduce them by recycling. But the real thrust was to make things easier by having an order number from the start -- I was just following up on a suggestion you'd made in a number of threads.

2. I guess 10 minutes seems more reasonable -- but I'd even suggest 5. I take your point about needless complexity, but I do think people will want to be able to control this -- still, it's up to you. What about a hook_uc_checkout_expire(), so that it could at least be altered by a custom module? Maybe overkill. But you're definitely right that if this is kept, there needs to be an upper limit.

3. Aside from wasted orders, my real purpose here is to allow the cart contents to be altered during checkout (at least by ajax, i know it's problematic to do it after all the 'process' callbacks have been called for the last time).

4. OK, I'll wrap this one as a separate issue.

The rules event for a timed-out order is a great idea - perhaps with a default configuration that marks the order as canceled (I think that's better than deleting it, bc store owners might want to know how many orders are being initiated and then abandoned). Likewise with the cron processing. I'll work up a patch for that too if you like.

xibun’s picture

subscribing

xibun’s picture

very interested in patch to sub-issue 1.

-> to me this is mostly a question of philosophy. from bank transfer module point of view it would be nicer to have the order number asap - but only about 10% of Ubercart shops use the bank transfer module. this is why I'm happy to go with what ever the project lead decides.
-> I'm also happy to give that patch a test ride - but again, I think first the philosophy of order numbers should be clear.
-> finally when it comes to "wasted order numbers" - personally I don't have an issue with them. I don't see a cost attached to them in the real world.

wodenx’s picture

I agree with everything xibun said. It's marginally easier for a contrib module to deal with line-items if there is an order number from the start, but not an insurmountable issue to handle $order_id==0 as a special case. So whatever project leads decide is fine with me.

As to wasted/non-sequential order numbers, I like longwave's suggestion of a separate invoice number which would only be incremented when an order was actually submitted. One way to implement would be a new 'uc_order_invoices' table mapping an auto incremented invoice_id to order_id.

Island Usurper’s picture

Status: Needs review » Fixed

I like the idea of putting expired orders into the Canceled status. We could even make an Abandoned status to differentiate them from explicit cancellations.

Ten minutes is reasonable enough, and should be enough for almost anybody. If there's enough demand, we can always add it in. I'd suggest a select box with options from 0 to 30 minutes instead of a textfield, though.

Committed the patch series, since it'll be easier to work on top of them that way. Please go back through and update issues that might be affected by this change. I know they're all connected with links, but I don't think I'd find them all.

xibun’s picture

tested sub-issues 1&3. works great. thanks!

about "abandoned" status: I would be in favour of it. helps to understand non-conversions.

wodenx’s picture

Status: Fixed » Closed (fixed)

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