Generate unique slugs for multiple users with the same name - #570 #573

#574

.gitignore

@@ -7,6 +7,7 @@ upload
 *.csv
 backups/
 *.log
+*.log.*
 
 # Python runtime and setup
 __pycache__

lowfat/models.py

@@ -1,5 +1,6 @@
 from datetime import datetime, date, timedelta
 import hashlib
+import itertools
 import re
 import uuid
 
@@ -8,6 +9,7 @@
 from constance import config
 
 import django.utils
+import django.utils.text
 from django.conf import settings
 from django.db import models
 from django.urls import reverse
@@ -159,13 +161,6 @@ def fix_url(url):
 
     return url
 
-def slug_generator(forenames, surname):
-    """Generate slug for Claimant"""
-    return "{}-{}".format(
-        forenames.lower().replace(" ", "-"),
-        surname.lower().replace(" ", "-")
-    )
-
 def pair_fund_with_blog(funds, status=None):
     """Create list of tuples where first element is fund and second is list of blog related with it."""
     args = {}
@@ -437,6 +432,36 @@ class Meta:
     added = models.DateTimeField(auto_now_add=True)
     updated = models.DateTimeField(auto_now=True)
     history = HistoricalRecords()
+
+    def get_absolute_url(self):
+        return reverse('claimant-slug-resolution', kwargs={'claimant_slug': self.slug})
+
+    def slug_generator(self):
+        """
+        Generate slug for Claimant - checking that it doesn't conflict with an existing Claimant.
+        """
+        base_slug = django.utils.text.slugify("{0}-{1}".format(self.forenames, self.surname))
+        slug = base_slug
+
+        for i in itertools.count():
+            try:
+                # Has this slug already been used?
+                existing = Claimant.objects.get(slug=slug)
+                if existing.pk == self.pk:
+                    break
+
+            except Claimant.DoesNotExist:
+                # No - use this slug
+                break
+
+            except Claimant.MultipleObjectsReturned:
+                # Yes - multiple times - try the next one
+                pass
+
+            # Yes - try the next one
+            slug = '{0}-{1}'.format(base_slug, i)
+
+        return slug
 
     def save(self, *args, **kwargs):  # pylint: disable=arguments-differ
         if not self.id:
@@ -446,7 +471,9 @@ def save(self, *args, **kwargs):  # pylint: disable=arguments-differ
                 config.FELLOWSHIP_EXPENSES_END_DAY
             )
 
-        self.slug = slug_generator(self.forenames, self.surname)
+        if not self.slug:
+            self.slug = self.slug_generator()
+
         self.website = fix_url(self.website)
         self.website_feed = fix_url(self.website_feed)
 

lowfat/views.py

@@ -239,7 +239,9 @@ def claimant_form(request):
         claimant.update_latlon()
         messages.success(request, 'Profile saved.')
         claimant_profile_update_notification(claimant)
-        return HttpResponseRedirect(reverse('my_profile'))
+        return HttpResponseRedirect(
+            reverse('claimant_slug', args=[claimant.slug])
+        )
 
     # Show submission form.
     context = {
@@ -341,17 +343,25 @@ def _claimant_detail(request, claimant):
 def claimant_detail(request, claimant_id):
     raise Http404("URL not supported in lowFAT 2.x.")
 
+
 def claimant_slug_resolution(request, claimant_slug):
-    """Resolve claimant slug and return the details."""
+    """
+    Resolve claimant slug and return the details.
+    """
     try:
         claimant = Claimant.objects.get(slug=claimant_slug)
-    except:  # pylint: disable=bare-except
-        claimant = None
-
-    if claimant:
         return _claimant_detail(request, claimant)
 
-    raise Http404("Claimant does not exist.")
+    except Claimant.DoesNotExist:
+        raise Http404('Claimant does not exist')
+
+    except Claimant.MultipleObjectsReturned:
+        message = 'Multiple claimants exist with the same slug identifier "{0}".' \
+                  'Please contact an admin to fix this.'.format(claimant_slug)
+
+        logger.error(message)
+        raise Http404(message)
+
 
 @login_required
 def my_profile(request):