Drop upper_charge logic for weight based shipping methods

Previously, weight based shipping methods accepted an upper charge
field. When no weight band matched the baskets weight, this was
returned. I am not aware of a single retailer who calculates shipping
charges like that. It's impossible to pick a sensible value for a
retailer, as there's no upper limit to the amount of items ordered, and
any shipping charges would have to cover costs for that.

docs/source/releases/v0.8.rst

@@ -392,6 +392,7 @@ Migrations
 * Shipping:
 
     - ``0007`` - Change ``WeightBand.upper_limit`` from ``FloatField`` to ``DecimalField``
+    - ``0008`` - Drop ``WeightBased.upper_charge`` field.
 
 .. _deprecated_features_in_0.8:
 

oscar/apps/shipping/abstract_models.py

@@ -83,12 +83,6 @@ class AbstractOrderAndItemCharges(AbstractBase):
 
 
 class AbstractWeightBased(AbstractBase):
-    upper_charge = models.DecimalField(
-        _("Upper Charge"), decimal_places=2, max_digits=12, null=True,
-        validators=[MinValueValidator(D('0.00'))],
-        help_text=_("This is the charge when the weight of the basket "
-                    "is greater than all the weight bands"))
-
     # The attribute code to use to look up the weight of a product
     weight_attribute = 'weight'
 

oscar/apps/shipping/migrations/0008_auto__del_field_weightbased_upper_charge.py

@@ -0,0 +1,62 @@
+# -*- coding: utf-8 -*-
+from south.utils import datetime_utils as datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        # Deleting field 'WeightBased.upper_charge'
+        db.delete_column(u'shipping_weightbased', 'upper_charge')
+
+
+    def backwards(self, orm):
+        # Adding field 'WeightBased.upper_charge'
+        db.add_column(u'shipping_weightbased', 'upper_charge',
+                      self.gf('django.db.models.fields.DecimalField')(null=True, max_digits=12, decimal_places=2),
+                      keep_default=False)
+
+
+    models = {
+        u'address.country': {
+            'Meta': {'ordering': "('-display_order', 'name')", 'object_name': 'Country'},
+            'display_order': ('django.db.models.fields.PositiveSmallIntegerField', [], {'default': '0', 'db_index': 'True'}),
+            'is_shipping_country': ('django.db.models.fields.BooleanField', [], {'default': 'False', 'db_index': 'True'}),
+            'iso_3166_1_a2': ('django.db.models.fields.CharField', [], {'max_length': '2', 'primary_key': 'True'}),
+            'iso_3166_1_a3': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '3', 'blank': 'True'}),
+            'iso_3166_1_numeric': ('django.db.models.fields.PositiveSmallIntegerField', [], {'null': 'True', 'db_index': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+            'printable_name': ('django.db.models.fields.CharField', [], {'max_length': '128'})
+        },
+        u'shipping.orderanditemcharges': {
+            'Meta': {'ordering': "['name']", 'object_name': 'OrderAndItemCharges'},
+            'code': ('oscar.models.fields.autoslugfield.AutoSlugField', [], {'allow_duplicates': 'False', 'max_length': '128', 'separator': "u'-'", 'blank': 'True', 'unique': 'True', 'populate_from': "'name'", 'overwrite': 'False'}),
+            'countries': ('django.db.models.fields.related.ManyToManyField', [], {'symmetrical': 'False', 'to': u"orm['address.Country']", 'null': 'True', 'blank': 'True'}),
+            'description': ('django.db.models.fields.TextField', [], {'blank': 'True'}),
+            'free_shipping_threshold': ('django.db.models.fields.DecimalField', [], {'null': 'True', 'max_digits': '12', 'decimal_places': '2', 'blank': 'True'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '128'}),
+            'price_per_item': ('django.db.models.fields.DecimalField', [], {'default': "'0.00'", 'max_digits': '12', 'decimal_places': '2'}),
+            'price_per_order': ('django.db.models.fields.DecimalField', [], {'default': "'0.00'", 'max_digits': '12', 'decimal_places': '2'})
+        },
+        u'shipping.weightband': {
+            'Meta': {'ordering': "['method', 'upper_limit']", 'object_name': 'WeightBand'},
+            'charge': ('django.db.models.fields.DecimalField', [], {'max_digits': '12', 'decimal_places': '2'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'method': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'bands'", 'to': u"orm['shipping.WeightBased']"}),
+            'upper_limit': ('django.db.models.fields.DecimalField', [], {'max_digits': '12', 'decimal_places': '3'})
+        },
+        u'shipping.weightbased': {
+            'Meta': {'ordering': "['name']", 'object_name': 'WeightBased'},
+            'code': ('oscar.models.fields.autoslugfield.AutoSlugField', [], {'allow_duplicates': 'False', 'max_length': '128', 'separator': "u'-'", 'blank': 'True', 'unique': 'True', 'populate_from': "'name'", 'overwrite': 'False'}),
+            'countries': ('django.db.models.fields.related.ManyToManyField', [], {'symmetrical': 'False', 'to': u"orm['address.Country']", 'null': 'True', 'blank': 'True'}),
+            'default_weight': ('django.db.models.fields.DecimalField', [], {'default': "'0.000'", 'max_digits': '12', 'decimal_places': '3'}),
+            'description': ('django.db.models.fields.TextField', [], {'blank': 'True'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '128'})
+        }
+    }
+
+    complete_apps = ['shipping']

oscar/templates/oscar/dashboard/shipping/weight_based_detail.html

@@ -49,10 +49,6 @@
                     <th>{% trans "Default product weight (kg)" %}</th>
                     <td>{{ method.default_weight }}</td>
                 </tr>
-                <tr>
-                    <th>{% trans "Upper charge" %}</th>
-                    <td>{{ method.upper_charge|currency }}</td>
-                </tr>
             </tbody>
         </table>
 
@@ -82,13 +78,6 @@
                             </td>
                         </tr>
                     {% endfor %}
-                    <tr>
-                        <td>{{ method.max_upper_limit|floatformat:3 }} - &infin;</td>
-                        <td>{{ method.upper_charge|currency }}</td>
-                        <td>
-                            <a href="{% url 'dashboard:shipping-method-edit' pk=method.pk %}" class="btn btn-primary">{% trans "Edit" %}</a>
-                        </td>
-                    </tr>
                 </tbody>
             </table>
         {% else %}

oscar/templates/oscar/dashboard/shipping/weight_based_list.html

@@ -31,7 +31,6 @@
                 <tr>
                     <th>{% trans "Name" %}</th>
                     <th>{% trans "Number of weight bands" %}</th>
-                    <th>{% trans "Upper charge" %}</th>
                     <th>{% trans "Default weight (kg)" %}</th>
                     <th></th>
                 </tr>
@@ -43,7 +42,6 @@
                             <a href="{% url 'dashboard:shipping-method-detail' pk=method.pk %}">{{ method.name }}</a>
                         </td>
                         <td>{{ method.num_bands }}</td>
-                        <td>{{ method.upper_charge|currency }}</td>
                         <td>{{ method.default_weight }}</td>
                         <td>
                             <a href="{% url 'dashboard:shipping-method-detail' pk=method.pk %}" class="btn">{% trans "View" %}</a>

tests/functional/dashboard/shipping_tests.py

@@ -15,7 +15,6 @@ class TestShippingMethodDashboard(WebTestCase):
         # Create a shipping method
         create_page = list_page.click(linkid="create_new_shipping_method")
         create_page.form['name'] = 'My method'
-        create_page.form['upper_charge'] = '25.00'
         detail_page = create_page.form.submit().follow()
         self.assertInContext(detail_page, 'method')
         self.assertEqual(1, models.WeightBased.objects.all().count())

tests/integration/shipping/model_method_tests.py

@@ -164,14 +164,14 @@ class WeightBasedMethodTests(TestCase):
         self.assertEqual(D('0.00'), charge.incl_tax)
         self.assertTrue(charge.is_tax_known)
 
-    def test_correct_charge_returned(self):
+    def test_simple_shipping_cost_scenario_handled_correctly(self):
         basket = newfactories.BasketFactory()
         product_attribute_value = newfactories.ProductAttributeValueFactory(
-            value_float=0.5)
+            value_float=2.5)
         basket.add_product(product_attribute_value.product)
 
-        expected_charge = D('4.00')
-        self.standard.bands.create(upper_limit=1, charge=expected_charge)
+        expected_charge = D('3.00')
+        self.standard.bands.create(upper_limit=3, charge=expected_charge)
         charge = self.standard.calculate(basket)
 
         self.assertEqual(expected_charge, charge.excl_tax)

tests/unit/shipping/model_tests.py

@@ -8,23 +8,12 @@ from oscar.apps.shipping import models
 
 class TestWeightBasedMethod(TestCase):
 
-    def test_doesnt_allow_negative_upper_charge(self):
-        method = models.WeightBased(
-            name="Dummy", upper_charge=D('-12.00'))
-        with self.assertRaises(ValidationError):
-            method.full_clean()
-
     def test_doesnt_allow_negative_default_weights(self):
         method = models.WeightBased(
-            name="Dummy", upper_charge=D('0.00'),
-            default_weight=D('-0.1'))
+            name="Dummy", default_weight=D('-0.1'))
         with self.assertRaises(ValidationError):
             method.full_clean()
 
-    def test_max_upper_limit_defaults_to_none(self):
-        method = models.WeightBased()
-        self.assertIsNone(method.max_upper_limit)
-
 
 class TestWeightBand(TestCase):