Skip to content
This repository has been archived by the owner on Apr 15, 2020. It is now read-only.

Randomly the order number is skipped and another one is attached #4

Closed
boldenamsterdam opened this issue Oct 10, 2017 · 10 comments
Closed

Comments

@boldenamsterdam
Copy link

Sometimes the order number is skipped.
In the attached screenshot order number 20170262 is created before(!) 20170261.
Order number 20170262 should actually be 20170260. Furthermore order number 20170260 is nowhere to find.
Any suggestion where it goes wrong?

friendly order numbers ordered by order number desc

@bossanova808
Copy link
Owner

Well that's odd. (The date stuff is done by Craft not the plugin really. And not used really... I've never really looked at it tbh)

Is this on a live site? Could be something weird to do with concurrency if you have lots of orders flowing through I suppose. As long as they have different numbers if the timing is quite concurrent it shouldn't prove a big problem I guess? Still....

A skip will occur if an order fails in another onBefore handler, or there's an issue with completing. Because the number has been taken and won't be re-used. That seems reasonable behaviour to me.

Anyway late here and I am on phone... Will look properly tomorrow!

@boldenamsterdam
Copy link
Author

boldenamsterdam commented Oct 10, 2017

Yes it is on production but there are hardly more than 2 orders per day. Therefore no issue with concurrency.
I checked also the logs but I could not find something. I will check if there is a fail on another onBefore handler.. maybe another plugin fails?

I was also thinking about a technical solution in the plugin.
A double check in what order number has actually been assigned before increasing the counter.

@bossanova808
Copy link
Owner

Ok, so I am pretty sure this is all to do with an order failing to complete. In a nutshell:

User comes along and tries to complete an order - gets an order number 260 but the order fails to complete for some reason. This number is now used up.

Someone else comes along and makes an order and gets 261

The original person comes back and now completes their order -> the plugin now gives them 262 but the dateCreated is older because the order itself was previously create - however the dateUpdated is later (as would be expected). The behaviour here will vary depending on the cart duration you have set, too (i.e. whether or not when the user comes back they get the same cart/order back, or have to start a new one).

I'd say the simple fix here might be to add a check that there isn't already an order number value on the order. That would at least mean the order started earlier would get the earlier number...but I am not actually sure that is better than a skip and the order completed later getting the later number..indeed, I think I prefer the current behaviour. I am not bothered by the odd skip in numbers personally as long as the numbers are client friendly and increasing as orders are made.

What do you think? I'm inclined to leave it as is really.

@boldenamsterdam
Copy link
Author

Ok I understand. Personally I don't mind skipping numbers either :) but client(s) use the numbers as a reference for financial administration and skipping invoice number(s) is a problem ..

dateCreated and dateUpdated are actually not the issue at all. They can be whatever. But what is important in my opinion is that there should be no gap in the numbers. So, I think just checking if a number is attached to an order before increasing the number is sufficient..

@bossanova808
Copy link
Owner

I guess I could make it a setting. I like my numbers to reflect the order received, but I can see your client's preference could make sense.

Feel free to do a PR (with the default of the setting being the as is behaviour)... Or I can do it sometime soon.

@boldenamsterdam
Copy link
Author

Yes sure I will.
Thanks

@boldenamsterdam
Copy link
Author

Could you create a branch or allow me to create one? I am not able to create one.

@bossanova808
Copy link
Owner

As in, you forked the repo and it wouldn't let you make a branch on your fork?

Anyway, I will make a dev branch

@sm9
Copy link

sm9 commented Dec 6, 2017

Just came across this issue as we too have a client whose finance department doesn't like missing order numbers in the sequence. Did you guys manage to sort out the new feature to prevent this or is it still work in progress?

@bossanova808
Copy link
Owner

There is a PR here - #5 - just want to confirm it's been properly tested and will then merge and do a release. Feel free to try it out yourself an post feedback on your testing on the PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants