Hello,

I'm hitting a severe issue, let's start in order: first, the context.

I have a definitely really custom checkout flow to implement, so I started by enabling the commerce_checkout module. Second step was to implement hook_commerce_order_states_info_alter() and hook_commerce_status_info_alter() in order to set-up my workflow steps depending on my business matter. Third step was to implement hook_commerce_checkout_panes_info() and hook_commerce_checkout_pages_info() to fit those steps and add the missing business steps (really a few).

Then, the problem I hit:

When I submit the Checkout button on the cart display, I always end up directly with a message such as "You must add products into your basket", which is really disturbing. And yes, my cart has been emptied. So I check the orders admin page, and see my cart being now an order with the "Canceled" state.

The problem, after introspecting a bit, is that I altered really deep the states and statuses, as the hooks allow me to do, but the commerce framework, pretty much everywhere, doesn't care about the user/module set workflow and still set hardcoded order statuses, such as there (commerce_checkout.module, around line 217):

function commerce_checkout_line_item_views_form_submit($form, &$form_state) {
  // Set the order status to the first checkout page.
  $order = commerce_order_load($form_state['order']->order_id);
  $form_state['order'] = commerce_order_status_update($order, 'checkout_checkout', TRUE);

In my case, checkout_checkout was a bad name, and I removed it, replacing it with an other state name (that fit in my business semantic). This hardcoded step induces this behavior: I am redirected onto the checkout menu item, it goes crazy because the checkout step doesn't exist anymore, so cannot be run, and my order ends up being canceled.

The first checkout step should be configurable or be dynamically deterministic so that custom workflow wouldn't suffer the same problem as I am experiencing now, OR this first meta-step could be fixed after the alteration hook is being called, but in all cases, right now there is an undocumented hardcoded behavior that contradicts the framework abilities.

CommentFileSizeAuthor
#7 api_function_to_determine_the_first_checkout_page-1417414-7.patch2.76 KBIcanDivideBy0

Comments

pounard’s picture

pounard
Primary language French
commented

PS: I checked on the 1.2 code and also in the actual git state, this specific piece of code is still there.

pounard’s picture

pounard
Primary language French
commented

The code that makes the commerce_checkout_router() menu callback going crazy is due to the commerce_checkout_page_access() function that actually checks that the current status is a checkout state: because the checkout_checkout status doesn't exist anymore, it does have a state determined so the user cannot access the page.

This is a guess, I didn't run a xdebug on it, but I'm quite sure this is it. I'm going to restore the checkout_checkout step as a meta-step in my business workflow as a quick fix, hope that it will work.

rszrama’s picture

rszrama commented
Title: Inconsistency when submiting the "checkout" button, saving orders with a potential non existing status » Add an API function to determine the "first" checkout page
Version: 7.x-1.1 » 7.x-1.x-dev
Category: bug » feature

Yeah, what you're bumping up against is the default expected behavior of the Checkout module. I don't see us changing the way it works, described as such: every checkout page has an associated order status automatically defined by the Checkout module. These statuses are used in checking access for a user to view a checkout page for a particular order, and they also determine whether or not the order is still considered a shopping cart for the purpose of price refreshing.

If you want to alter the way the checkout works and use different statuses, you cannot just alter the state and statuses involved. Instead you should provide alternate checkout pages or simply alter the title of the checkout pages w/o changing the machine-names. It could be worth considering us changing behavior to look for whatever the first checkout page's status is, in which case you would just be able to delete the initial page and replace it with something else. Statuses would still be prefixed with checkout_, but you would be free to change what the first page was.

So, I've retitled this issue accordingly, and of course the second part of adding this API function will be to make sure that any redirection to the "Start" of checkout uses this API function. : )

pounard’s picture

pounard
Primary language French
commented

Hum I'm balancing between "bug report" and "feature request" because I did implement all the hooks as expected: the fact it is actually legal in an API point of view to remove the checkout_checkout status entry really makes everything break.

rszrama’s picture

rszrama commented

Well, it's also "legal" to remove all the menu items defined by our modules, but then nothing will work either. ; )

I think as designed, the system works as expected; the primary Checkout page cannot be reorganized outside of an alter hook. We have to balance between intended use and possible use, imo, with extending possible uses moving us into the realm of new features.

(If it helps, bug or feature request, I still intend to put it in. It's a very reasonable idea. : )

pounard’s picture

pounard
Primary language French
commented

Yes of course, but the whole module code is oriented towards the alteration of every bit of it, it's a huge generic machine, and in this particular case a single hardcoded string actually do make everything collapse, and in that sense, I consider it as a bug.

Thanks for your answer thought, still fair enought :)

IcanDivideBy0’s picture

IcanDivideBy0 commented
StatusFileSize
new api_function_to_determine_the_first_checkout_page-1417414-7.patch2.76 KB

Here is a patch to add such an API function (commerce_checkout_page_get_first).
Also added a fix to the commerce_checkout_line_item_views_form_submit (see #1424078: Cart does not redirect to default checkout status) and commerce_checkout_form_cancel_submit functions, and setting the default checkout status according to the first page.

There still need to check if the is any other hard coded 'checkout_checkout', event if a grep -irn 'checkout_checkout' . gives me nothing.

IcanDivideBy0’s picture

IcanDivideBy0 commented
Status: Active » Needs review
vmd111’s picture

vmd111 commented

#7: api_function_to_determine_the_first_checkout_page-1417414-7.patch queued for re-testing.

rszrama’s picture

rszrama commented
Issue tags: +1.3 review

Tagging.

helior’s picture

helior commented
Status: Needs review » Reviewed & tested by the community

Finally got around to wrapping my head around the issue, checkout module, and the patch provided. All looks well, thanks @IcanDivideBy0

rszrama’s picture

rszrama commented
Status: Reviewed & tested by the community » Fixed

Great work, committed! Welcome to the committers log, Samuel. : )

Also, thanks for your work on the Paybox module. I had to do an integration with Paybox a couple years ago on Ubercart and had a rough time at it. Didn't help that I don't speak French, but really I just remember their whole system being pretty archaic. Glad you were able to get it released, and hope you haven't lost too much sleep to it. ; )

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