Ensure user addresses are unique per user

This change adds a uniqueness constraint on user addresses to prevent
duplicates.  A few changes follow:

- The user address forms in account and checkout are modified to take a
  user as a constructor arg so uniqueness can be checked.

- The assignment of a user when creating a new address has been moved
  into the form.

oscar/apps/address/abstract_models.py

@@ -189,7 +189,7 @@ class AbstractCountry(models.Model):
         ordering = ('-display_order', 'name',)
 
     def __unicode__(self):
-        return self.printable_name
+        return self.printable_name or self.name
 
 
 class AbstractShippingAddress(AbstractAddress):
@@ -285,6 +285,7 @@ class AbstractUserAddress(AbstractShippingAddress):
         verbose_name = _("User address")
         verbose_name_plural = _("User addresses")
         ordering = ['-num_orders']
+        unique_together = ('user', 'hash')
 
 
 class AbstractBillingAddress(AbstractAddress):

oscar/apps/address/forms.py

@@ -1,11 +1,13 @@
 from django.conf import settings
-from django.forms import ModelForm
+from django import forms
+from django.utils.translation import ugettext_lazy as _
+from django.forms.models import construct_instance
 from django.db.models import get_model
 
 UserAddress = get_model('address', 'useraddress')
 
 
-class AbstractAddressForm(ModelForm):
+class AbstractAddressForm(forms.ModelForm):
 
     def __init__(self, *args, **kwargs):
         """
@@ -23,3 +25,29 @@ class UserAddressForm(AbstractAddressForm):
     class Meta:
         model = UserAddress
         exclude = ('user', 'num_orders', 'hash', 'search_text')
+
+    def __init__(self, user, *args, **kwargs):
+        self.user = user
+        super(UserAddressForm, self).__init__(*args, **kwargs)
+
+    def clean(self):
+        # Check this isn't a duplicate of another address
+        cleaned_data = super(UserAddressForm, self).clean()
+        candidate = construct_instance(
+            self, self.instance, self._meta.fields)
+        qs = self._meta.model.objects.filter(
+            user=self.user,
+            hash=candidate.generate_hash())
+        if self.instance.id:
+            qs = qs.exclude(id=self.instance.id)
+        if qs.count() > 0:
+            raise forms.ValidationError(_(
+                "This address is already in your addressbook"))
+        return cleaned_data
+
+    def save(self, commit=True):
+        instance = super(UserAddressForm, self).save(commit=False)
+        instance.user = self.user
+        if commit:
+            instance.save()
+        return instance

oscar/apps/address/migrations/0006_auto__add_unique_useraddress_hash_user.py

@@ -0,0 +1,92 @@
+# -*- coding: utf-8 -*-
+import datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        # Adding unique constraint on 'UserAddress', fields ['hash', 'user']
+        db.create_unique('address_useraddress', ['hash', 'user_id'])
+
+
+    def backwards(self, orm):
+        # Removing unique constraint on 'UserAddress', fields ['hash', 'user']
+        db.delete_unique('address_useraddress', ['hash', 'user_id'])
+
+
+    models = {
+        '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', [], {'max_length': '3', 'null': 'True', 'db_index': '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'})
+        },
+        'address.useraddress': {
+            'Meta': {'ordering': "['-num_orders']", 'unique_together': "(('user', 'hash'),)", 'object_name': 'UserAddress'},
+            'country': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['address.Country']"}),
+            'date_created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
+            'first_name': ('django.db.models.fields.CharField', [], {'max_length': '255', 'null': 'True', 'blank': 'True'}),
+            'hash': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'is_default_for_billing': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'is_default_for_shipping': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'last_name': ('django.db.models.fields.CharField', [], {'max_length': '255', 'blank': 'True'}),
+            'line1': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+            'line2': ('django.db.models.fields.CharField', [], {'max_length': '255', 'null': 'True', 'blank': 'True'}),
+            'line3': ('django.db.models.fields.CharField', [], {'max_length': '255', 'null': 'True', 'blank': 'True'}),
+            'line4': ('django.db.models.fields.CharField', [], {'max_length': '255', 'null': 'True', 'blank': 'True'}),
+            'notes': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}),
+            'num_orders': ('django.db.models.fields.PositiveIntegerField', [], {'default': '0'}),
+            'phone_number': ('django.db.models.fields.CharField', [], {'max_length': '32', 'null': 'True', 'blank': 'True'}),
+            'postcode': ('django.db.models.fields.CharField', [], {'max_length': '64', 'null': 'True', 'blank': 'True'}),
+            'search_text': ('django.db.models.fields.CharField', [], {'max_length': '1000'}),
+            'state': ('django.db.models.fields.CharField', [], {'max_length': '255', 'null': 'True', 'blank': 'True'}),
+            'title': ('django.db.models.fields.CharField', [], {'max_length': '64', 'null': 'True', 'blank': 'True'}),
+            'user': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'addresses'", 'to': "orm['auth.User']"})
+        },
+        'auth.group': {
+            'Meta': {'object_name': 'Group'},
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}),
+            'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'})
+        },
+        'auth.permission': {
+            'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'},
+            'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '50'})
+        },
+        'auth.user': {
+            'Meta': {'object_name': 'User'},
+            'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}),
+            'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+            'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}),
+            'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'})
+        },
+        'contenttypes.contenttype': {
+            'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"},
+            'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '100'})
+        }
+    }
+
+    complete_apps = ['address']

oscar/apps/checkout/mixins.py

@@ -190,7 +190,7 @@ class OrderPlacementMixin(CheckoutSessionMixin):
             address.populate_alternative_model(shipping_addr)
             return shipping_addr
         else:
-            raise AttributeError("No shipping address data found")
+            raise UnableToPlaceOrder("No shipping address data found")
 
     def update_address_book(self, user, shipping_addr):
         """

oscar/apps/checkout/views.py

@@ -181,23 +181,17 @@ class UserAddressCreateView(CheckoutSessionMixin, CreateView):
     """
     Add a USER address to the user's addressbook.
 
-    This is not the same as creating a SHIPPING Address, although if used for the order,
-    it will be converted into a shipping address at submission-time.
+    This is not the same as creating a SHIPPING Address, although if used for
+    the order, it will be converted into a shipping address at submission-time.
     """
     template_name = 'checkout/user_address_form.html'
     form_class = UserAddressForm
 
-    def get_context_data(self, **kwargs):
-        kwargs = super(UserAddressCreateView, self).get_context_data(**kwargs)
-        kwargs['form_url'] = reverse('checkout:user-address-create')
+    def get_form_kwargs(self):
+        kwargs = super(UserAddressCreateView, self).get_form_kwargs()
+        kwargs['user'] = self.request.user
         return kwargs
 
-    def form_valid(self, form):
-        self.object = form.save(commit=False)
-        self.object.user = self.request.user
-        self.object.save()
-        return self.get_success_response()
-
     def get_success_response(self):
         messages.info(self.request, _("Address saved"))
         # We redirect back to the shipping address page
@@ -212,11 +206,11 @@ class UserAddressUpdateView(CheckoutSessionMixin, UpdateView):
     form_class = UserAddressForm
 
     def get_queryset(self):
-        return UserAddress._default_manager.filter(user=self.request.user)
+        return self.request.user.addresses.all()
 
-    def get_context_data(self, **kwargs):
-        kwargs = super(UserAddressUpdateView, self).get_context_data(**kwargs)
-        kwargs['form_url'] = reverse('checkout:user-address-update', args=(str(kwargs['object'].id),))
+    def get_form_kwargs(self):
+        kwargs = super(UserAddressUpdateView, self).get_form_kwargs()
+        kwargs['user'] = self.request.user
         return kwargs
 
     def get_success_url(self):

oscar/apps/customer/views.py

@@ -536,6 +536,11 @@ class OrderLineView(DetailView, PostActionMixin):
         messages.info(self.request, msg)
 
 
+# ------------
+# Address book
+# ------------
+
+
 class AddressListView(ListView):
     """Customer address book"""
     context_object_name = "addresses"
@@ -552,17 +557,16 @@ class AddressCreateView(CreateView):
     mode = UserAddress
     template_name = 'customer/address_form.html'
 
+    def get_form_kwargs(self):
+        kwargs = super(AddressCreateView, self).get_form_kwargs()
+        kwargs['user'] = self.request.user
+        return kwargs
+
     def get_context_data(self, **kwargs):
         ctx = super(AddressCreateView, self).get_context_data(**kwargs)
         ctx['title'] = _('Add a new address')
         return ctx
 
-    def form_valid(self, form):
-        self.object = form.save(commit=False)
-        self.object.user = self.request.user
-        self.object.save()
-        return HttpResponseRedirect(self.get_success_url())
-
     def get_success_url(self):
         messages.success(self.request, _("Address saved"))
         return reverse('customer:address-list')
@@ -573,17 +577,22 @@ class AddressUpdateView(UpdateView):
     model = UserAddress
     template_name = 'customer/address_form.html'
 
+    def get_form_kwargs(self):
+        kwargs = super(AddressUpdateView, self).get_form_kwargs()
+        kwargs['user'] = self.request.user
+        return kwargs
+
     def get_context_data(self, **kwargs):
-        ctx =  super(AddressUpdateView, self).get_context_data(**kwargs)
+        ctx = super(AddressUpdateView, self).get_context_data(**kwargs)
         ctx['title'] = _('Edit address')
         return ctx
 
     def get_queryset(self):
-        return UserAddress._default_manager.filter(user=self.request.user)
+        return self.request.user.addresses.all()
 
     def get_success_url(self):
         messages.success(self.request, _("Address saved"))
-        return reverse('customer:address-detail', kwargs={'pk': self.get_object().pk })
+        return reverse('customer:address-list')
 
 
 class AddressDeleteView(DeleteView):
@@ -597,6 +606,11 @@ class AddressDeleteView(DeleteView):
         return reverse('customer:address-list')
 
 
+# ------------
+# Order status
+# ------------
+
+
 class AnonymousOrderDetailView(DetailView):
     model = Order
     template_name = "customer/anon_order.html"

oscar/templates/oscar/checkout/user_address_form.html

@@ -13,7 +13,7 @@
 
 {% block shipping_address %}
     <h1>{% trans "Edit address" %}</h1>
-    <form action="{{ form_url }}" method="post" class="form-horizontal">
+    <form action="." method="post" class="form-horizontal">
         <div class="well well-info">
             {% csrf_token %}
             {% include "partials/form_fields.html" with form=form %}

tests/integration/address/form_tests.py

@@ -0,0 +1,37 @@
+from django.test import TestCase
+from django.contrib.auth.models import User
+from django_dynamic_fixture import G
+
+from oscar.apps.address import models, forms
+
+
+class TestUserAddressForm(TestCase):
+
+    def setUp(self):
+        self.user = G(User)
+        self.country = models.Country.objects.create(
+            iso_3166_1_a2='GB', name="UNITED KINGDOM")
+
+    def test_merges_addresses_with_same_hash(self):
+        data = {
+            'user': self.user,
+            'first_name': "Matus",
+            'last_name': "Moravcik",
+            'line1': "1 Egg Street",
+            'line4': "London",
+            'postcode': "N1 9RE",
+            'country': self.country}
+
+        # Create two addresses, which are slightly different
+        models.UserAddress.objects.create(**data)
+        other = data.copy()
+        other['first_name'] = 'Izidor'
+        duplicate = models.UserAddress.objects.create(**other)
+
+        # Edit duplicate to be same as original and check that the two
+        # addresses are merged when the form saves.
+        post_data = data.copy()
+        post_data['country'] = self.country.iso_3166_1_a2
+        form = forms.UserAddressForm(self.user, post_data, instance=duplicate)
+        self.assertFalse(form.is_valid())
+        self.assertTrue(len(form.errors['__all__']) > 0)