Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow order numbers to be incremental with prefix and suffix #636

Closed
havenswift-hosting opened this Issue Jun 23, 2015 · 101 comments

Comments

Projects
None yet
5 participants
@havenswift-hosting
Copy link

havenswift-hosting commented Jun 23, 2015

See feature request: https://features.cubecart.com/topic/incremental-order-numbers

I know that this says it is experimental but I believe it should be taken out until completed as it doesnt work correctly.

It is possible to place an order and the order numbering works OK, the order gets sent through to whichever gateway you want but returns with the old style order numbers and so order isnt updated from Pending to processing ; there is a mix of what order numbers have been used between the orders listing (incremental) and the transactions (old) and worse still the product code details seem to be lost from the order after payment - you can see an order with detail lines and prices but no product details

@abrookbanks abrookbanks added the bug label Jun 24, 2015

@abrookbanks abrookbanks added this to the 6.0.7 milestone Jun 24, 2015

@abrookbanks abrookbanks self-assigned this Jun 24, 2015

@abrookbanks

This comment has been minimized.

Copy link
Member

abrookbanks commented Jun 24, 2015

Thanks.

@abrookbanks

This comment has been minimized.

Copy link
Member

abrookbanks commented Jun 24, 2015

See commit b4e9ae4

@Dirty-Butter

This comment has been minimized.

Copy link

Dirty-Butter commented Jun 24, 2015

FYI - I have been using Chuggyskin's Incremental Order Numbers mod for years very successfully. Perhaps you can work something out with him to make it stock??????

@NaturalSigns

This comment has been minimized.

Copy link

NaturalSigns commented Feb 20, 2016

Has this now been abandoned?. Its a shame as in the UK you have to invoice with a sequential number (to my understanding, see HMRC guidelines [http://www.hmrc.gov.uk/manuals/vatrecmanual/vatrec5020.htm] ). + sequential seems to be the logical approach for a clean store install.

@Dirty-Butter

This comment has been minimized.

Copy link

Dirty-Butter commented Feb 21, 2016

I still use the v5 code from Chuggyskins (at some point modified by Bsmither). I cannot, however, merge the mod code with CC code anymore - I have to comment out the newer order number code sections in 6.0.10 to use the mod.

@Dirty-Butter

This comment has been minimized.

Copy link

Dirty-Butter commented Feb 12, 2018

Referencing your last comment with images on #1867. I like the idea of keeping the randomized REFERENCE to the order, while the human (admin and customer) sees an incremental order number. I have a very low volume store, so my incremental modification based on ChuggySkins works well for me. I suspect a large volume store might run into problems with a true incremental order number system that is easily human read. Good luck with this!!!

@abrookbanks

This comment has been minimized.

Copy link
Member

abrookbanks commented Feb 12, 2018

Thanks. I think my new approach will work. I'm only making changes to the views and not any mechanics.

abrookbanks added a commit that referenced this issue Feb 12, 2018

@Dirty-Butter

This comment has been minimized.

Copy link

Dirty-Butter commented Feb 12, 2018

Sounds good! I would suggest that the admin be able to set the initial number of the sequence. For instance, my last order number was 2018-PC-4019. If I changed to your system right now, I would want the next order to be 4020. (the year turns over automatically with Chuggyskins, and I set the PC for plushcatalog - I also have the choice of starting numbering over for the new year or continuing with the sequence. I chose continuing.

abrookbanks added a commit that referenced this issue Feb 12, 2018

abrookbanks added a commit that referenced this issue Feb 12, 2018

@abrookbanks abrookbanks added feature request and removed bug labels Feb 12, 2018

@abrookbanks

This comment has been minimized.

Copy link
Member

abrookbanks commented Feb 12, 2018

Duplicate of #193

@abrookbanks abrookbanks marked this as a duplicate of #193 Feb 12, 2018

@PloughGuy

This comment has been minimized.

Copy link

PloughGuy commented Feb 13, 2018

I have been working on a version of this recently. (I will now stop).
For maximum flexibility, and to knock over other development requests in the queue, I was planning to allow the user to specify some decorations:

  • A static prefix stored as a global that, if defined, is applied before the number (this is a development request.)
  • A static suffix stored as a global that, if defined, is applied after the number. (Not requested, if we are doing a prefix, we should do a suffix for symmetry, if nothing else.)
  • a number of digits and a leading filler (zeros, for example, or unicorn emojis.)
  • a next order number initialization value. This would allow a store to seed from order numbers other than 1 or to prefix the order number with the year (201800000001) or to migrate from some other platform with an existing order book. It should be possible to update this at any time.

Benefits of prefix and suffix: Useful for people with multiple stores that want to transfer transactions to a central accounting system.
Benefit of fixed length numbers - predictable formatting.
Benefit of starting with any number - new number range for each year, (e.g. 1000000 this year, 2000000 next year.) - seeding 000132000 disguises that your store opened yesterday - makes it easy to move into CC from some other store with order number continuity.

If this was implemented using hooks, alternate order numbering schemes can be implemented as required. The Elboonian digits-in-reverse-order requirement, for example, can be met with relataive ease.

An Implementation Test
Finally, I will share some SQL. This is the core of my solution. You are all smart folk, and I am sure you have already thought of this. However, I have heard rumblings in the ethers that a database-resident solution will impose performance problems. I have tested this, and it works a treat, is very fast and fully atomic. There are performance numbers below, and scripts to help you confirm it.

// Read the current value to an SQL variable, and add 1 to it for next time. This operation is atomic.
// The table always contains the next order number.
$mysqli->query("UPDATE next_order_number SET next_order_number = (@order_number := next_order_number) + 1 WHERE number_name = 'next_order_number'");

// Extract the SQL variable, which is local to this session.
$result = $mysqli->query("SELECT @order_number;");
$row = $result->fetch_assoc();
$order_number = $row['@order_number'];

// Use it.
printf("
next order number is %d",$order_number)

Performance Tests
Running it at the command line level from a shell script, I get 10,000 numbers in just over three seconds, for better than 3000 order numbers per second, atomically sourced from an innodb table.
-------------------------------8<-------------------------------
RussMBP17:test_sequential_orders russ$ time ./runem.sh 1
threads is 1
Localhost via UNIX socket
Running 10000 iterations
next order number is 119142

real 0m3.076s
user 0m0.300s
sys 0m0.359s
RussMBP17:test_sequential_orders russ$
-------------------------------8<-------------------------------
Note the hostname - this is running under OS X on a 2012 MacBook Pro 17", which I know you all covet.

  • OS X 10.13.3
  • 4 cores - so 8 processors with hyperthreading
  • 16 GB RAM

Lets scale it up: Eight parallel threads.
-------------------------------8<-------------------------------
RussMBP17:test_sequential_orders russ$ time ./runem.sh 8
threads is 8
threads is 7
threads is 6
threads is 5
threads is 4
threads is 3
threads is 2
threads is 1
Localhost via UNIX socket
Running 10000 iterations
Localhost via UNIX socket
Running 10000 iterations
Localhost via UNIX socket
Running 10000 iterations
Localhost via UNIX socket
Running 10000 iterations
Localhost via UNIX socket
Running 10000 iterations
Localhost via UNIX socket
Running 10000 iterations
Localhost via UNIX socket
Running 10000 iterations
Localhost via UNIX socket
Running 10000 iterations
next order number is 109021
next order number is 109056
next order number is 109089
next order number is 109102
next order number is 109119
next order number is 109135
next order number is 109140
next order number is 109142

real 0m12.716s
user 0m3.206s
sys 0m3.447s
-------------------------------8<-------------------------------
We get 80,000 sequential order numbers from 8 threads in under 13 seconds or 6,153 per second. CPU utilization is about 40%,.

Ramping it up to 32 concurrent threads for 320,000 order numbers, I get:
-------------------------------8<-------------------------------
real 0m12.416s
user 0m3.156s
sys 0m3.512s
-------------------------------8<-------------------------------
This gives 26,000 order numbers per second. The time improves, presumably, because the PHP and MySQL initializations are amortised over a much larger sample. Once again, CPU load peaked at 40%, presumably limited by the speed of the SSD.

I expect the load to connect to the database, save the order_summary and order_inventory rows, and update stock levels for the order would well eclipse this, so the cost of managing sequential order numbers in a database table will not impose an onerous load, even for very-high-throughput stores.

Conclusion: We have nothing to fear from storing the next available number in a database table like grown-ups should, and using appropriate SQL to retrieve it atomically.

If someone has a more modern bit of hardware to run it on (2015, perhaps?) then the numbers would be interesting. Still not buying a new Mac until this one dies, my spare dies, and no more 17" MBPs are available on eBay.

The scripts are uploaded so you can play with it yourself. Remove the .txt from the end. runem,sh takes a single parameter which is the number of threads. The php program contains a constant which sets the number of iterations.

A rudimentary check for duplicates is to confirm that the largest reported number of a run is appropriately larger than the result at the start.

[runem.sh.txt]
(https://github.com/cubecart/v6/files/1718525/runem.sh.txt)
test_sequential_orders.php.txt

The table was created as follows:
-------------------------------8<-------------------------------
CREATE TABLE next_order_number (
number_id int(10) NOT NULL AUTO_INCREMENT COMMENT 'Primary Key - in case we need more numbers later',
number_name text COLLATE utf8_unicode_ci NOT NULL COMMENT 'Next order''s order number',
next_order_number int(10) NOT NULL,
PRIMARY KEY (number_id)
) ENGINE=InnoDB AUTO_INCREMENT=2 DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci
-------------------------------8<-------------------------------

@PloughGuy

This comment has been minimized.

Copy link

PloughGuy commented Feb 13, 2018

I believe Amazon uses those crazy order numbers because they have vast numbers of loosely coupled online stores for scalability and availability reasons. Each store generates its own distinct and scrambled order numbers to prevent duplication, which include redundancy coding and other security features.

The system CubeCart currently uses has a 1/10000 chance of generating a duplicate for two orders arriving in the same second. It is probably due to the relatively low traffic (two orders every hour would be good) that this has not caused problems already.

My preference would be to replace the existing order number solution completely with hooks and offer some choices. I bet the uptake on the long order numbers would be very low...

@PloughGuy

This comment has been minimized.

Copy link

PloughGuy commented Feb 13, 2018

Dirty-butter said

I suspect a large volume store might run into problems with a true incremental order number system that is easily human read

I work with SAP systems that can process thousands of orders per second. They use sequential order numbers (because its the law) and have no problems with it. Each of the application servers in a system grabs a couple of hundred numbers from the database in a chunk and issues them until depleted, whereupon it grabs a bunch more. I expect that unless Amazon itself wants to switch to CC, we will have no problems.

There is no reason to not go sequential except (in our case) effort. I have demonstrated the throughput above on an ancient laptop. Currently available XEON servers will do much better.

@abrookbanks

This comment has been minimized.

Copy link
Member

abrookbanks commented Feb 13, 2018

Thanks for the very detailed response. I've started building this from the CubeCart_order_summary auto incremented id column. I'll code it so this column can be changed. The easiest approach would be to create a new column with stored procedure to format itbased on the auto incremented id column.

E.G. {prefix}{leading zeros}{auto increment id + starting integer}{postfix}.

@PloughGuy

This comment has been minimized.

Copy link

PloughGuy commented Feb 22, 2018

So that implies you solves Dirty’s problem then?

You can’t just leave a bloke hangin’...

@abrookbanks

This comment has been minimized.

Copy link
Member

abrookbanks commented Feb 22, 2018

No not at all. Please see my comment above. I'm waiting for access to check it. It will be reopened if needs be.

@Dirty-Butter

This comment has been minimized.

Copy link

Dirty-Butter commented Feb 22, 2018

Sorry - Ian helped me setup an FTP account for you, but I can't figure out how to make it work on CuteFTP. I misunderstood and thought you were satisfied it was MY error on the install. Will message you the ftp info as best I can.

@abrookbanks

This comment has been minimized.

Copy link
Member

abrookbanks commented Feb 22, 2018

Yes please I'd like to test your store.

@abrookbanks

This comment has been minimized.

Copy link
Member

abrookbanks commented Feb 23, 2018

Reopening because I just hit me that some mysql users might not be able to create triggers. A check needs to be made that the trigger set worked and revert if not.

@abrookbanks abrookbanks reopened this Feb 23, 2018

@PloughGuy

This comment has been minimized.

Copy link

PloughGuy commented Feb 23, 2018

The alternative solution that I nicked from the interwebs earlier then benchmarked-and-tested (using a table and an atomic SQL statement) might help out here...

@PloughGuy

This comment has been minimized.

Copy link

PloughGuy commented Feb 23, 2018

The table can be created as part of the upgrade to 6.2.

@abrookbanks

This comment has been minimized.

Copy link
Member

abrookbanks commented Feb 23, 2018

Sounds a bit over complicated when we can use a simple trigger. If the MySQL user doesn't have permission to read/write triggers then they can be granted in cPanel or by the hosting company.

@PloughGuy

This comment has been minimized.

Copy link

PloughGuy commented Feb 23, 2018

@PloughGuy

This comment has been minimized.

Copy link

PloughGuy commented Feb 23, 2018

Question for Al: if the trigger disappears, does the order creation method detect it and fix it? If not, will the store start dropping (probably all) transactions because of duplicate keys?

@abrookbanks

This comment has been minimized.

Copy link
Member

abrookbanks commented Feb 23, 2018

I literally can't see any reason at all why it would disappear. I suppose if you move hosting and you imported the database and the user didn't have permissions to create the trigger it would get lost. Thats not really a concern for CubeCart though.

Are you saying your hosting company doesn't allow you to use triggers?

@PloughGuy

This comment has been minimized.

Copy link

PloughGuy commented Feb 23, 2018

I expect they allow me to use triggers. I can do a science experiment to find out. Just saying that code in files and data in tables are relatively well understood by the general public, but triggers, not so much. I am merely exploring hardening options.

@abrookbanks

This comment has been minimized.

Copy link
Member

abrookbanks commented Feb 23, 2018

I would have thought triggers would be the cleanest way and the merchant/developer shouldn't need to know anything about them as CubeCart handles it all for them. I do need to make it fool proof though and make sure the trigger was created successfully.

@PloughGuy

This comment has been minimized.

Copy link

PloughGuy commented Feb 23, 2018

Foolproof is the word. Evolution is making better fools every day...

@PloughGuy

This comment has been minimized.

Copy link

PloughGuy commented Feb 23, 2018

If the Lord does choose to taketh away the trigger, do we get a duo key error? I presume it will create one order with the default orderId then the second one will fail. Is that correct?

If the field is NOT NULL and the trigger vanishes, what happens? Can we detect this error and check that the trigger is OK then try again? This would be self-healing without a cost to each order other than the success check, which I hope already happens.

So something like “If the insert fails, recreate the trigger and try one more time.”

@abrookbanks

This comment has been minimized.

Copy link
Member

abrookbanks commented Feb 23, 2018

I'll test it out and see. However once a trigger is made it should just stay. I can't see any reason for the lord to taketh it away.

abrookbanks added a commit that referenced this issue Feb 23, 2018

@Dirty-Butter

This comment has been minimized.

Copy link

Dirty-Butter commented Feb 23, 2018

The Stability and performance improvement fixed my problem. THANKS Al!! And I was correct that using zero as the Increment Increase continues my existing sequence on the test site. It is definitely going to be an issue if the admin saves the page after preview and then realizes they want to save it again with a different value. I DO still know how to decode base64 config, wipe out a value, and then encode with what I want. Many would not. You do have that super powerful and potentially catastrophic section in maintenance that you're trusting everyday admins to respect - maybe you should consider giving admins a chance for a redo on the Increment Increase?

abrookbanks added a commit that referenced this issue Feb 23, 2018

abrookbanks added a commit that referenced this issue Feb 23, 2018

@abrookbanks

This comment has been minimized.

Copy link
Member

abrookbanks commented Feb 23, 2018

Ok. Happy and closing again.

@PloughGuy

This comment has been minimized.

Copy link

PloughGuy commented Feb 23, 2018

Al said: I'll test it out and see. However once a trigger is made it should just stay. I can't see any reason for the lord to taketh it away.

He moves in mysterious ways, especially when it comes to computerised solutions, which he seems to find personally offensive. "I created the universe, life, DNA and evolution in seven days. It has worked a treat for over four thousand years. You guys have been working on IT for 70 years and look - I poke here and over it goes... Ha! Created in His own image indeed! Who came up with that one?"

If there is a way for it to go wrong, it will. David Mitchell as Shakespeare should have said: "A groat's-worth of hardening doth buy a guinea-worth of leisure." But he didn't. Still a good quote though.

@abrookbanks

This comment has been minimized.

Copy link
Member

abrookbanks commented Feb 24, 2018

Thinking about this.. it should carry on working as it's still reliant on the traditional order I'd format. Then once the trigger is put back any empty fields will be populated. In the mean time some blank order numbers will be seen. Not a disaster. Any number of things can go wrong with CubeCart. Bridge will need to be crossed when it does.

@PloughGuy

This comment has been minimized.

Copy link

PloughGuy commented Feb 24, 2018

Our thoughts and prayers are with it...

Good call. At the first sign of trouble, though, we have a potential solution in the recipe book.
Get back to the packing...
R

abrookbanks added a commit that referenced this issue Feb 27, 2018

@abrookbanks abrookbanks changed the title Incremental Order Numbers not working Allow order numbers to be incremental with prefix and suffix Mar 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.