Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Parameters encoding and explicit shipping_method order amount consistency #1

Merged
merged 6 commits into from

2 participants

@jmaupetit
  • Fix encoding errors from urlencode with french unicode strings
  • We need to update the total amount of the order (i.e. incl charges) when a single shipping method is proposed

For my custom shipping methods, I needed to bind the shipping address to the shipping method (charges depend on that parameter). I think it pay be of interest for oscar ...

See paypal/express/__init__.py:241

shipping_method.set_shipping_addr( shipping_address )
jmaupetit added some commits
@jmaupetit jmaupetit Recursive add files 358ecb2
@jmaupetit jmaupetit merge from trunk ff2de86
@jmaupetit jmaupetit ignore build 852532e
@jmaupetit jmaupetit Fixed typos 340cb09
@jmaupetit jmaupetit Unicode (french) strings must be properly encoded for urlencode. When…
… a single shipping method has been defined we must update the total amount of the order. We need to bind shipping address to shipping method for some shipping methods (Colissimo and France Express cases).
d14d2fc
@codeinthehole codeinthehole commented on the diff
.gitignore
((4 lines not shown))
+integration.py
+build/
+django_oscar_paypal.egg-info/
@codeinthehole Owner

I have these in my global gitignore file as you end up adding these lines to every .gitignore file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
paypal/express/__init__.py
@@ -233,15 +236,18 @@ def set_txn(basket, shipping_methods, currency, return_url, cancel_url, update_u
params['L_SHIPPINGOPTIONNAME%d' % index] = method.name
params['L_SHIPPINGOPTIONAMOUNT%d' % index] = charge
+ # Set shipping charge explicitly if it has been passed
+ if shipping_method:
+ shipping_method.set_shipping_addr( shipping_address )
@codeinthehole Owner

As this method isn't in oscar, I can't really merge the pull request yet as it would break other installs.

One option would be to patch oscar to support this method but before I do that, I'd like to understand a little more about why you need to link the shipping method to a shipping address. Could you expand on that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jmaupetit

Some shipping methods rates depends on the shipping location (regions in a country or international shipping), hence, the shipping address is required. Am I wrong ?

@codeinthehole

The way I handle that is in the shipping repository, which returns a list of available methods based on the basket, user and shipping address. See https://github.com/tangentlabs/django-oscar/blob/master/oscar/apps/shipping/repository.py You should be able to handle your requirements by providing your own repository and overriding the get_shipping_methods method. You can make it return different methods depending on the shipping address.

If you do this, then you shouldn't need to bind the shipping address to the method. Let me know if this works for you.

@jmaupetit

You are right. I did this in add_basket_to_methods called by get_shipping_methods. It does not seems to work.

@codeinthehole codeinthehole merged commit d3fde64 into django-oscar:master
@codeinthehole

Thanks for that - now merged in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 24, 2012
  1. @jmaupetit

    Recursive add files

    jmaupetit authored
  2. @jmaupetit

    merge from trunk

    jmaupetit authored
Commits on May 3, 2012
  1. @jmaupetit

    ignore build

    jmaupetit authored
  2. @jmaupetit

    Fixed typos

    jmaupetit authored
  3. @jmaupetit

    Unicode (french) strings must be properly encoded for urlencode. When…

    jmaupetit authored
    … a single shipping method has been defined we must update the total amount of the order. We need to bind shipping address to shipping method for some shipping methods (Colissimo and France Express cases).
Commits on May 4, 2012
  1. @jmaupetit
This page is out of date. Refresh to see the latest.
View
4 .gitignore
@@ -1,3 +1,5 @@
htmlcov
dist
-integration.py
+integration.py
+build/
+django_oscar_paypal.egg-info/
@codeinthehole Owner

I have these in my global gitignore file as you end up adding these lines to every .gitignore file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
View
3  MANIFEST.in
@@ -1 +1,2 @@
-include *.rst
+include *.rst
+recursive-include paypal/express/templates/paypal *.txt *.html
View
13 paypal/express/__init__.py
@@ -52,6 +52,9 @@ def _fetch_response(method, extra_params):
'SIGNATURE': settings.PAYPAL_API_SIGNATURE,
}
params.update(extra_params)
+ for k in params.keys():
+ if type(params[k]) == unicode:
+ params[k] = params[k].encode('utf-8')
payload = urllib.urlencode(params.items())
# Make request
@@ -233,15 +236,17 @@ def set_txn(basket, shipping_methods, currency, return_url, cancel_url, update_u
params['L_SHIPPINGOPTIONNAME%d' % index] = method.name
params['L_SHIPPINGOPTIONAMOUNT%d' % index] = charge
+ # Set shipping charge explicitly if it has been passed
+ if shipping_method:
+ max_charge = charge = shipping_method.basket_charge_incl_tax()
+ params['PAYMENTREQUEST_0_SHIPPINGAMT'] = charge
+ params['PAYMENTREQUEST_0_AMT'] += charge
+
# Both the old version (MAXAMT) and the new version (PAYMENT...) are needed
# here - think it's a problem with the API.
params['PAYMENTREQUEST_0_MAXAMT'] = amount + max_charge
params['MAXAMT'] = amount + max_charge
- # Set shipping charge explicitly if it has been passed
- if shipping_method:
- params['PAYMENTREQUEST_0_SHIPPINGAMT'] = shipping_method.basket_charge_incl_tax()
-
# Handling set to zero for now - I've never worked on a site that needed a
# handling charge.
params['PAYMENTREQUEST_0_HANDLINGAMT'] = D('0.00')
View
8 setup.py
@@ -12,6 +12,14 @@
license='BSD',
platforms=['linux'],
packages=find_packages(),
+ include_package_data = True,
+ # See http://pypi.python.org/pypi?%3Aaction=list_classifiers
+ classifiers=['Environment :: Web Environment',
+ 'Framework :: Django',
+ 'Intended Audience :: Developers',
+ 'License :: OSI Approved :: BSD License',
+ 'Operating System :: Unix',
+ 'Programming Language :: Python'],
install_requires=['django-oscar>=0.2',
'requests',
'purl'],
Something went wrong with that request. Please try again.