Using OrderItem.data in price calculation

[bianchimro]

Hi and thanks for the the work on this project, we're trying to adopt it as our base for doing ecommerce sites with Django.

We have an use case where we want to calculate the price of a product dynamically using properties passed by the user on the order form.

To accomplish this we tried to leverage the "data" parameter passed to the modify_item method of the Order class, but the method sets data on the item after the call to product.get_price

data is set here: https://github.com/matthiask/plata/blob/next/plata/shop/models.py#L449

call to get_price is done here: https://github.com/matthiask/plata/blob/next/plata/shop/models.py#L437

So in practice data passed as modify_item parameter is not usable to calculate the price, as it's set after the price calculation.

We resolved our use case by defining our Order model and swapping order of the two calls, but this probably something that should be ported to the base Order implementation.

Do you see any side effect with this?

Best regards Mauro

[matthiask]

Hi,

I probably had a similar issue, and decided to let get_price and get_prices return the base prices, and add upgrade prices only in Product.handle_order_item. In my case:

    def handle_order_item(self, orderitem):
        upgrades = set()
        for upgrade in orderitem.data.get('upgrades', []):
            orderitem._unit_price += upgrade['price_excl_tax']
            orderitem._unit_tax += upgrade['tax']
            upgrades.add(upgrade['id'])

        orderitem.name = unicode(self)
        orderitem.sku = self.generate_sku(upgrades)

(This is from a webshop selling computer hardware; it's possible to select more RAM, a faster SSD etc. when adding a product to the cart.)

Anyway, I don't think assigning item.data earlier would be a problem, but I'm not 100% sure! It's probably even better, because otherwise a get_price implementation looking at item.data would use stale data in the worst case.

Anyway. The general idea was that get_price cannot return the final price anyway, because the price might depend on discounts, might depend on the current user in case of educational discounts, might depend on other items in the cart etc.

[bianchimro]

Hi @matthiask , thanks for your quick response, I understand it better now (I am using the handle_order_item already for adding accessories that are not sellable alone).

In my case, I prefer to create different prices for my products based on their properties. I am selling prints of different sizes and different materials, and creating a price instance for every combination I am interested in. (as the relation might be non linear or difficult to express by configuration)

My implementation of get_price looks like this:

def get_price(self, currency=None, orderitem=None):

        if currency is None:
            currency = (
                orderitem.currency if orderitem else
                plata.shop_instance().default_currency())

        try:
            return self.prices.get(
                currency=currency,
                size=orderitem.data.get('size', None),
                material=orderitem.data.get('material', None))

        except IndexError:
            raise self.prices.model.DoesNotExist

I swapped calls of price calculation and item data setting as I mentioned before in my Order model. (And if this is not impacting in other places I'd change it in the default implementation, seems more consistent anyway. If you want i can make a PR for this)

Also, in my order form I only show combinations of size and material for which I have a price.

I think the current API is good enough as it allows multiple implementation, probably we should add some more complex examples like this or similar ones.

Thanks again, regards Mauro

[matthiask]

Mauro, it seems to me that you might want to take a closer look at https://github.com/matthiask/plata-options-product/blob/master/options_product/models.py

[bianchimro]

Hi Matthias, thanks for the link, It's very useful and probably will solve my use case. I'll surely take some ideas from there.

If you agree I'd keep this issue open, as I think my original proposal of swapping the two methods calls is still worth investigating.

[matthiask]

That's alright. Do you want to submit a pull request? I'll probably merge this change right away if Travis CI doesn't complain.