It is possible to skip steps by manipulating the URL

[mbrochh]

When seeing the cart, you can just enter /ship/flat/ as a URL and click at the submit button. This will obviously result in the following error:

ValueError at /shop/ship/flat/process/
Cannot assign None: "ExtraOrderPriceField.order" does not allow null values.

How do we handle this? Does the shop need to be aware of the current step and redirect to that step in case someone manipulates the URL?

[bmihelac]

I would say that we need order_required decorator.

[mbrochh]

Hmm sounds like a good idea!

[alesdotio]

we should put the decorator on the payment views as well

[alesdotio]

@stephenmuss how exactly should the callable be used?

If i do this

@on_method(order_required(some_function))

I get

AttributeError
'function' object has no attribute 'user'
/django-shop/shop/util/order.py in get_order_from_request
12.    if request.user and not isinstance(request.user, AnonymousUser): 

[stephenmuss]

@alesdotio it should be a url path.

You could pass in a raw string or better to use django.core.urlresolvers.reverse.

e.g.

@on_method(order_required(reverse('shop_welcome')))

Note: Not tested. Just off the top of my head.

Edit: OK. So I didn't think about this at the time, but of course using reverse here won't work. Which means the decorator in its current state will only take a url path as a string.

Some refactoring may be warranted. A simple lazy reverse could do the trick.

I'm happy to take a look if you'd like. What's your preference in terms of the optional argument supplied? Leave it as a url or would you prefer to pass in a callable instead?

[stephenmuss]

I've just made a commit to my fork which approaches it slightly different.

So now we could use @on_method(order_required('shop_welcome'))

Perhaps that would be more flexible? Happy to open a pull request.

I'd still need to rework it so that args and kwargs etc can be passed in the same as in django.core.urlresolvers.reverse.

[alesdotio]

@stephenmuss
Having both options would probably be best. I thought that was the whole point of the if callable(redirect_url) part. If you could make that work that would be great, even if you have to call reverse through another function (which is what I tried to do)

def get_cart_url():
    return reverse('cart')

@on_method(order_required(get_cart_url))
def some_view()...

[stephenmuss]

@alesdotio
The if callable(redirect_url) was to allow it to be used with a default value for redirect_url which just redirected to '/'.

However, I agree that being able to pass in a callable or just a string might be a nicer way to do it.

Making the developer explicitly pass in redirect_url is probably a better idea anyway.

I'll make some changes and open another pull request.

Cheers.

[alesdotio]

awesome, thank you