Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Currently, the Order class sets an order number to its ID in its postSave() hook, if it's not already set, regardless if it was placed or not, so by other means on first save of the cart.
When you want to ensure sequential and ascending order numbers, which is for sure a very common wish, you'll need to override this number. In general not a problem, you could hook into the place transition and set it to any other number.
But with the current implementation, you'll always have to take care that your "real" order numbers do not interfere with the temporary ones. That means, you can never start with 1... instead you must prefix, suffix or start at order number 1000000, etc..
Talked to mglaman on IRC. He agrees that this is a PITA for him too.
First question: is it really necessary to always have an order number for carts? mglman means: "an order _should_ have a number.. whether draft or not because admin based workflows"
How can we get around that?
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | refactor_order_number_generation-2842356-7.patch | 6.43 KB | agoradesign |
Comments
Comment #2
bojanz CreditAttribution: bojanz at Centarro for Adapt commentedI believe that in 1.x carts didn't have order numbers.
An order should get a number when it's placed.
So we need to kill the preSave() part + do #2730131: Add a commerce_number_pattern submodule for sequential (order/invoice) number generation.
Comment #3
agoradesign CreditAttribution: agoradesign commentedWe don't have to implement the other issue necessarily in the same step, do we?
Means, that we could refactor the architecture and provide a default order number generation, which points to the order id - or maybe already do a sequential numbering - but leave the configurable part aside and extend that within the other issue.
Should we do that like the price resolvers, so that it can easily be swapped?
Comment #4
agoradesign CreditAttribution: agoradesign commentedHere's the patch. It removes the code from the postSave() hook and moves the logic to a place transition subscriber (with a low priority to be easily overridable by custom module) -> from now on, draft orders will no longer have an order number set by default.
Github PR to follow...
Comment #5
agoradesign CreditAttribution: agoradesign commentedSorry, uploaded the wrong patch... that one was beta4 compatible, this is the right one against dev
Comment #6
agoradesign CreditAttribution: agoradesign commentedhttps://github.com/drupalcommerce/commerce/pull/609
Comment #7
agoradesign CreditAttribution: agoradesign commentedupdated patch to be in sync with the Github PR. No functional changes, only fixing OrderUserTest and added OrderNumberTest
Comment #8
agoradesign CreditAttribution: agoradesign commentedbtw, I've started building a module for generating order numbers, based upon this patch. It's sandbox currently, as I'll have to finish configuration, add a proper description, write tests,... But the functionality does work already: https://www.drupal.org/sandbox/agoradesign/2845422
Comment #9
bojanz CreditAttribution: bojanz at Centarro for Adapt commented@agoradesign
Make sure to look at the commerce_billy logic, that's the one we want to replicate.
Comment #10
agoradesign CreditAttribution: agoradesign commentedOk, I will... currently that is only partially covered. But I'm gonna to refactor the whole module anyway, coz I'm not satisfied with the current architecture anyway
Comment #12
bojanz CreditAttribution: bojanz at Centarro commentedThanks!