DocDataOrder <-> Oscar Order relation

[jedie]

We have these models:

oscar.apps.order.models.Order
oscar_docdata.models.DocdataOrder

The order number connects both of them:

Order.number == DocdataOrder.merchant_order_id

Wouldn't it be better to have a direct Django ORM relation? This would make access easier and more effective.

Maybe that wasn't done specifically in the past. Why?

[maerteijn]

Probably the reason in the past is to prevent any tight coupling to the order model as all models in Oscar are abstract classes and made to be extendable / replaceable.

But I'd say that a foreign key would make sense.

Maybe @vdboor (the original author) can explain this ? 😃

[jedie]

Probably the reason in the past is to prevent any tight coupling to the order model as all models in Oscar are abstract classes and made to be extendable / replaceable.

I think that's no problem as long as you specify the target model as a string.

[vdboor]

Yes, it would be better to have a direct FK relation. I do think the number field should still be there, in case an docdata order can't be linked to an oscar order.

[jedie]

in case an docdata order can't be linked to an oscar order.

But in what case could that happen?

Actually, it's only possible if you delete one of them. But that should never be done, should it?

[vdboor]

Deleting orders is a strange thing, that's like deleting invoices. It should be an append-only table. Using on_delete=models.PROTECT would make sense to me.

in case an docdata order can't be linked to an oscar order.

Yeah indeed. I'm really not entirely sure why I didn't just create a FK. One option would be to perform the payment first, and then create the oscar order. That's still a possible implementation choice.

I did notice however, that for customer support it's way more useful to store the order first in a "draft" or "new" state, and then perform the payment. When something went wrong there, or the payment was made but somehow didn't ping back, you still have the original order information for customer support to work with. Otherwise the complete basket information is lost.

[maerteijn]

@jedie Do you see any direct benefits for changing this to a foreign key in your project? It is there now for almost 5 years and I did not notice any problems with it in the projects where I use this package.

Changing this would imply to create a data migration which could lead to implications when corresponding oscar orders don't exist or any other unforeseen issues. In the projects I maintain the 'advantage' of having a foreign key relation would not weigh up to the burden of a data migration and the consequences.

But this could be different in your case so I could reconsider my opinion.

[jedie]

I've noticed the problem while implementing the task to process all open and paid orders. For this I have to look in both models:

• My oscar order model has the status: "pending" / "processed"
• docdata order model has only the "paid" information

That was the reason for opening: https://github.com/django-oscar/django-oscar-docdata/issues/48 If the docdata order model has a status like "processed" or "shipped" then i can only get the "open&paid" list from there...

Now i do something like this (abbreviated code):

def get_oldest_open_docdata_order():
    """
    Returns paid&pending DocdataOrder with the oldest "updated" date.
    """
    oscar_order_qs = Order.objects.all().filter(status=settings.PIPELINE_STATUS_PENDING)
    pending_order_ids = tuple(oscar_order_qs.order_by("number").values_list("number", flat=True))

    docdata_orders = DocdataOrder.objects.all().order_by("updated").filter(
        status=DocdataOrder.STATUS_PAID
    ).filter(
        merchant_order_id__in=pending_order_ids
    )

    return docdata_orders[0]

[maerteijn]

The status of the oscar order should also change to 'paid' when the order is fully paid. This is also the case in the sandbox application so I don't see any reason why you need to check the Docdata orders?

[jedie]

The status of the oscar order should also change to 'paid' when the order is fully paid.

Hm! That's an interesting statement. Of course, that makes everything easier. But I didn't find it like that. I'm going to take another look at it.