search

django-oscar / django-oscar-docdata

Docdata Payments Gateway integration for django-oscar
Apache License 2.0
22 stars 11 forks source link

DocData order ID uniqueness #47

Closed jedie closed 5 years ago

jedie commented 5 years ago

As the README says:

Docdata is really keen on having unique merchant order ids

At least during testing it has happened to me several times that the ID is not unique. For this we have DOCDATA_ORDER_ID_START in the sandbox app...

First question is: Will this also become a problem in production? Second question is: How can we build unique IDs automatic? Without a offset?

The source order ID generates oscar in: oscar.apps.order.utils.OrderNumberGenerator it's just:

100000 + basket.id

Unfortunately, it looks like you can't just change/replace the OrderNumberGenerator in oscar, isn't it? The sandbox changed the ID in Facade.get_create_payment_args()

If i see it right: The so called merchant order id is merchantOrderReference here: https://github.com/django-oscar/django-oscar-docdata/blob/master/tests/testdata/xsd1-1_3.xsd#L2351

This string can be 255 characters long.

Wouldn't it be the easiest to do combine the current date with the basket.id ?

maerteijn commented 5 years ago

It won't become a problem in production unless you throw away your production database and start over. Order numbers should be unique in Oscar as well.

Note that I added the DOCDATA_ORDER_ID_START for testing purposes of the sandbox and should not be used in production.

Unfortunately, it looks like you can't just change/replace the OrderNumberGenerator in oscar, isn't it?

Oscar is designed to be fully customizable, not only the models but also most of the utility classes. There is an example for customizing the OrderNumberGenerator in oscar's docs: https://django-oscar.readthedocs.io/en/releases-1.6/topics/customisation.html#start-customising

This way you can generate the order numbers as you would like to.

The sandbox application customized the checkout app as described here

jedie commented 5 years ago

What's about to add a customizing the OrderNumberGenerator in the sandbox app and remove all of the DOCDATA_ORDER_ID_START stuff?

Just add a datetime string prefix to the order number. The goal: There are never the problem of conflicts while testing.

If that's what wanted, I'll make a pull request.

maerteijn commented 5 years ago

That would bring the burden of an extra application into the sandbox including extra maintenance and possible incompatibilities with (future) Oscar versions.

I'd like to keep the sandbox as minimal as possible. (In the future the checkout application could be removed as well). So I'd say to do this in your own project.

jedie commented 5 years ago

I'd like to keep the sandbox as minimal as possible

You're absolutely right.

What's about #50 ?