re-assign customer.selected_shipping_method in one_page_checkout view

[baffolobill]

Let's assume a shipping method depends on some criteria (country, city, etc.). On the cart page customer has selected "method1", but on the checkout page some criteria has been changed (for example, country) and updated list of the shipping methods was loaded. The updated list doesn't contain the "method1" and none of the methods is selected. The customer doesn't select the new one method and press "submit order" button. There are two major bugs in this case:

1. incorrect order total (order.price), because the old shipping method and shipping cost will be used for calculations;
2. incorrect shipping method will be assigned to the order.

[pigletto]

Which version of LFS do you have? Seems to me that this works properly in the latest version from the github repository

[baffolobill]

I have a fork based on v0.7. It works fine for me too. But I found that in one_page_checkout there is no check whether the value is set for the fields shipping_method & payment_method. I've added to a form:

class OnePageCheckoutForm(forms.Form):
...
    shipping_method = forms.CharField(required=True, max_length=2)
    payment_method = forms.CharField(required=True, max_length=2)
...

    def __init__(self, *args, **kwargs):
        self.request = kwargs.pop('request', None)
        super(OnePageCheckoutForm, self).__init__(*args, **kwargs)
        ...
        customer = customer_utils.get_customer(self.request)
        if customer:
            # Now I'm sure that shipping/payment method is valid
            lfs.shipping.utils.update_to_valid_shipping_method(self.request, customer)
            lfs.payment.utils.update_to_valid_payment_method(self.request, customer)
            customer.save()
            ...
            self.fields['shipping_method'].initial = customer.selected_shipping_method_id
            self.fields['payment_method'].initial = customer.selected_payment_method_id

I'll try to show you, why that's the possible bug (look at the comments in the code)

# lfs/checkout/views.py
def one_page_checkout(request, template_name="lfs/checkout/one_page_checkout.html"):
    ...
    return render_to_response(template_name, RequestContext(request, {
       ...
        "shipping_inline": shipping_inline(request),
        "payment_inline": payment_inline(request, bank_account_form),
       ...
    }))

...
def payment_inline(request, form, template_name="lfs/checkout/payment_inline.html"):
    ...
    selected_payment_method = lfs.payment.utils.get_selected_payment_method(request)
# there is no guarantee that the <selected_payment_method> will be in the list
    valid_payment_methods = lfs.payment.utils.get_valid_payment_methods(request)
    ...

def shipping_inline(request, template_name="lfs/checkout/shipping_inline.html"):
    ...
    selected_shipping_method = lfs.shipping.utils.get_selected_shipping_method(request)
# there is no guarantee that the <selected_shipping_method> will be in the list
    shipping_methods = lfs.shipping.utils.get_valid_shipping_methods(request)
    ...

# lfs/payment/utils.py
def get_selected_payment_method(request):
    customer = customer_utils.get_customer(request)
    if customer and customer.selected_payment_method:
# THIS VALUE MAY BE INVALID
# and as a result not selected in the rendered html.
# That's why I've made `payment_method` required in the OnePageCheckoutForm.
        return customer.selected_payment_method
    else:
        return get_default_payment_method(request)


# lfs/shipping/utils.py
def get_selected_shipping_method(request):
    customer = customer_utils.get_customer(request)
    if customer and customer.selected_shipping_method:
# The same situation is here.
        return customer.selected_shipping_method
    else:
        return get_default_shipping_method(request)

<!-- lfs/checkout/payment_inline.html -->
{% for payment_method in payment_methods %}
  ...
  <input type="radio"
        name="payment_method"
<!-- there is no guarantee that the `selected_payment_method` will be in the list -->
        {% ifequal payment_method.id selected_payment_method.id  %}checked="checked"{% endifequal %}
    ...
{% endfor %}