part-way though converting to slugs for people

9 files changed, 85 insertions, 44 deletions

diff --git a/core/models/troggle.py b/core/models/troggle.py
index 5db2d10..0977f84 100644
--- a/core/models/troggle.py
+++ b/core/models/troggle.py
@@ -80,14 +80,15 @@ class Person(TroggleModel):
 
     first_name = models.CharField(max_length=100)
     last_name = models.CharField(max_length=100)
-    fullname = models.CharField(max_length=200)
+    fullname = models.CharField(max_length=200) # display name, but should not be used for lookups
     nickname = models.CharField(max_length=200)
-    slug = models.SlugField(max_length=50, unique=True)
+    slug = models.SlugField(max_length=50, blank=True, null=True) # unique, enforced in code not in db
 
     is_vfho = models.BooleanField(
         help_text="VFHO is the Vereines für Höhlenkunde in Obersteier, a nearby Austrian caving club.",
         default=False,
     )
+    is_guest = models.BooleanField(default=False) # This is per-Person, not per-PersonExpedition
     mug_shot = models.CharField(max_length=100, blank=True, null=True)
     blurb = models.TextField(blank=True, null=True)
     orderref = models.CharField(max_length=200)  # for alphabetic
@@ -101,6 +102,7 @@ class Person(TroggleModel):
         ordering = ("orderref",)  # "Wookey" makes too complex for: ('last_name', 'first_name')
 
     def __str__(self):
+        return self.slug
         if self.last_name:
             return f"{self.first_name} {self.last_name}"
         return self.first_name
@@ -150,7 +152,7 @@ class PersonExpedition(TroggleModel):
     person = models.ForeignKey(Person, on_delete=models.CASCADE)
     slugfield = models.SlugField(max_length=50, blank=True, null=True)  # 2022 to be used in future
 
-    is_guest = models.BooleanField(default=False)
+    # is_guest = models.BooleanField(default=False) # This is per-Person, not per-PersonExpedition
 
     class Meta:
         ordering = ("-expedition",)
@@ -162,6 +164,16 @@ class PersonExpedition(TroggleModel):
 
     def get_absolute_url(self):
         # we do not use URL_ROOT any more.
+        return(f"/personexpedition/{self.person.slug}/{self.expedition.year}")
+        # why does this hang the system ?
+        return reverse(
+            "personexpedition",
+            kwargs={
+                "slug": self.slug,
+                "year": self.expedition.year,
+            },
+        )
+        # old style, no longer used
         return reverse(
                 "personexpedition",
                 kwargs={
diff --git a/core/views/logbooks.py b/core/views/logbooks.py
index d9e2192..10a9bee 100644
--- a/core/views/logbooks.py
+++ b/core/views/logbooks.py
@@ -193,6 +193,9 @@ def person(
         this_person = Person.objects.get(first_name=first_name, last_name=last_name)
     except:
         message = f"Person not found '{first_name} {last_name}' - possibly Scottish? (See our <a href=\"/handbook/troggle/namesredesign.html\">Proposal to fix this</a>)"
+        peeps = Person.objects.filter(first_name=first_name, last_name=last_name)
+        if len(peeps) > 1:
+            message = f"Multiple people ({len(peeps)}) with this name '{first_name} {last_name}' -  (See our <a href=\"/handbook/troggle/namesredesign.html\">Proposal to fix this</a>)"
         return render(request, "errors/generic.html", {"message": message})
 
     return render(request, "person.html", {"person": this_person})
@@ -232,8 +235,8 @@ def get_person_chronology(personexpedition):
     return res2
 
 
-def personexpedition(request, first_name="", last_name="", year=""):
-    person = Person.objects.get(first_name=first_name, last_name=last_name)
+def personexpedition(request, slug="", year=""):
+    person = Person.objects.get(slug=slug)
     this_expedition = Expedition.objects.get(year=year)
     personexpedition = person.personexpedition_set.get(expedition=this_expedition)
     personchronology = get_person_chronology(personexpedition)
diff --git a/core/views/scans.py b/core/views/scans.py
index 8e334bf..7e25862 100644
--- a/core/views/scans.py
+++ b/core/views/scans.py
@@ -142,7 +142,7 @@ def fixsurvextick(w, ticks):
     ticks["S"] = w.fixsurvextick(ticks["S"])
 
 
-def walletslistperson(request, first_name, last_name):
+def walletslistperson(request, slug):
     """Page which displays a list of all the wallets for a specific person
     HORRIBLE linear search through everything. Index and do SQL query properly
     """
@@ -163,20 +163,21 @@ def walletslistperson(request, first_name, last_name):
         return manywallets
 
     # print("-walletslistperson")
-
-    try:
-        if last_name:
-            p = Person.objects.get(fullname=f"{first_name} {last_name}")
-        else:
-            # special Wookey-hack
-            p = Person.objects.get(first_name=f"{first_name}")
-    except:
-        # raise
-        return render(
-            request,
-            "errors/generic.html",
-            {"message": f'Unrecognised name of a expo person: "{first_name} {last_name}"'},
-        )
+    p = Person.objects.get(slug=slug)
+    # try:
+
+        # if last_name:
+            # p = Person.objects.get(fullname=f"{first_name} {last_name}")
+        # else:
+            # # special Wookey-hack
+            # p = Person.objects.get(first_name=f"{first_name}")
+    # except:
+        # # raise
+        # return render(
+            # request,
+            # "errors/generic.html",
+            # {"message": f'Unrecognised name of a expo person: "{first_name} {last_name}"'},
+        # )
 
     manywallets = tickspersonwallet(p)
     expeditions = Expedition.objects.all()
diff --git a/parsers/people.py b/parsers/people.py
index 4799ebf..c0ffa10 100644
--- a/parsers/people.py
+++ b/parsers/people.py
@@ -3,9 +3,9 @@ import os
 import re
 from html import unescape
 from pathlib import Path
+from unidecode import unidecode
 
 from django.conf import settings
-from unidecode import unidecode
 
 from troggle.core.models.troggle import DataIssue, Expedition, Person, PersonExpedition
 
@@ -17,7 +17,9 @@ or they should use the same code by importing a module.
 
 
 def parse_blurb(personline, header, person):
-    """create mugshot Photo instance"""
+    """create mugshot Photo instance
+    Would be better if all this was done before the Person object was created in the db, then it would not
+    need re-saving (which is slow)"""
     ms_filename = personline[header["Mugshot"]]
     ms_path = Path(settings.EXPOWEB, "folk", ms_filename)
 
@@ -60,7 +62,19 @@ def parse_blurb(personline, header, person):
 
     person.save()
 
-
+slug_cache = {}
+def troggle_slugify(longname):
+    """Uniqueness enforcement too. Yes we have had two "Dave Johnson"s
+    """
+    slug = longname.strip().lower().replace(" ","-")
+    if len(slug) > 40: # slugfield is 50 chars
+        slug = slug[:40]
+    if slug in slug_cache: 
+        slug_cache[slug] += 1
+        slug = f"{slug}_{slug_cache[slug]}"
+    slug_cache[slug] = 1
+    return slug
+    
 def load_people_expos():
     """This is where the folk.csv file is parsed to read people's names.
     Which it gets wrong for people like Lydia-Clare Leather and various 'von' and 'de' middle 'names'
@@ -86,8 +100,11 @@ def load_people_expos():
 
     for personline in personreader:
         name = personline[header["Name"]]
-        name = re.sub(r"<.*?>", "", name)
-        slug = slugify(name)
+        name = re.sub(r"<.*?>", "", name) 
+        
+        match = re.match(r"^([^(]*)(\(([^)]*)\))?", name) # removes nickname in brackets
+        displayname = match.group(1)
+        slug = troggle_slugify(displayname)
 
         firstname = ""
         nick = ""
@@ -97,34 +114,39 @@ def load_people_expos():
         lastname = matchlastname.group(1).strip()
 
         splitnick = re.match(r"^([\w&;\s]+)(?:\(([^)]*)\))?", name)
-        fullname = splitnick.group(1)
-
-        nick = splitnick.group(2) or ""
+        fullname = splitnick.group(1) # removes Nickname in brackets, but also cuts hyphenated names
+        nick = splitnick.group(2) or "" 
 
         fullname = fullname.strip()
-        names = fullname.split(" ")
+        
+        names = fullname.split(" ") # This may have more than one, e.g. "Adeleide de Diesback"
         firstname = names[0]
         if len(names) == 1:
-            lastname = ""
+            lastname = "" # wookey special code
+            
+        #restore fullname to be the whole string
+        fullname = displayname
 
         if personline[header["VfHO member"]] == "":
             vfho = False
         else:
             vfho = True
 
-        coUniqueAttribs = {"first_name": firstname, "last_name": (lastname or "")}
-        otherAttribs = {"is_vfho": vfho, "fullname": fullname, "nickname": nick}
+        # would be better to just create the python object, and only cmmit to db once all done inc blurb
+        # and better to save all the Persons in a bulk update, then do all the PersonExpeditions
+        coUniqueAttribs = {"slug": slug}
+        otherAttribs = {"first_name": firstname, "last_name": (lastname or ""), "is_vfho": vfho, "fullname": fullname, "nickname": nick,"is_guest": (personline[header["Guest"]] == "1")}
         person = Person.objects.create(**otherAttribs, **coUniqueAttribs)
 
-        parse_blurb(personline=personline, header=header, person=person)
+        parse_blurb(personline=personline, header=header, person=person) # saves to db too
 
         # make person expedition from table
         for year, attended in list(zip(headers, personline))[5:]:
             expedition = Expedition.objects.get(year=year)
             if attended == "1" or attended == "-1":
                 coUniqueAttribs = {"person": person, "expedition": expedition}
-                otherAttribs = {"is_guest": (personline[header["Guest"]] == "1")}
-                pe = PersonExpedition.objects.create(**otherAttribs, **coUniqueAttribs)
+                # otherAttribs = {"is_guest": (personline[header["Guest"]] == "1")} 
+                pe = PersonExpedition.objects.create(**coUniqueAttribs)
     print("", flush=True)
 
 
diff --git a/templates/expedition.html b/templates/expedition.html
index c0b4724..c3685e4 100644
--- a/templates/expedition.html
+++ b/templates/expedition.html
@@ -43,7 +43,7 @@ an "<b>S</b>" for a survey trip.  The colours of the "<b>T</b>" and "<b>S</b>" a
 </tr>
 {% for personexpoday in personexpodays %}
   <tr>
-  <td><a href="{{ personexpoday.personexpedition.get_absolute_url }}">{{personexpoday.personexpedition.person|safe}}</a></td>
+  <td><a href="{{ personexpoday.personexpedition.get_absolute_url }}">{{personexpoday.personexpedition.person.fullname|safe}}</a></td>
   {% for activities in personexpoday.personrow %}
 
   {% if activities.personentries or activities.survexblocks %}
diff --git a/templates/person.html b/templates/person.html
index 274448b..ed5c70c 100644
--- a/templates/person.html
+++ b/templates/person.html
@@ -1,7 +1,7 @@
 {% extends "base.html" %}
 {% block title %}Person {{person}}{% endblock %}
 {% block contentheader %}
-<h2> {{person|safe}} </h2>
+<h2> {{person.fullname|safe}} </h2>
 {% endblock %}
 
 {% block content %}
@@ -15,7 +15,7 @@
 {% endif %}
 <br class="clearfloat" />
 
-<h3>{{person|safe}} has been on expo in the following years:</h3>
+<h3>{{person.fullname|safe}} has been on expo in the following years:</h3>
 
 <table>
 <th>Expo</th><th>Logbook mentions</th><th>Survex trips</th>
@@ -36,7 +36,7 @@
 {% endfor %}
 </table>
 <h3>Surveys done</h3>
-Wallets and surveys mentioning <a href="/wallets/person/{{person}}">{{person}}</a><br>
+Wallets and surveys mentioning <a href="/wallets/person/{{person}}">{{person.fullname}}</a><br>
 
 {% if person.blurb %}
 {{person.blurb|safe}}
diff --git a/templates/personexpedition.html b/templates/personexpedition.html
index 365c812..2b035eb 100644
--- a/templates/personexpedition.html
+++ b/templates/personexpedition.html
@@ -3,7 +3,7 @@
 
 {% block content %}
 <h1>
-  <a href="{{personexpedition.person.get_absolute_url}}">{{personexpedition.person|safe}}</a> : 
+  <a href="{{personexpedition.person.get_absolute_url}}">{{personexpedition.person.fullname|safe}}</a> : 
   <a href="{{personexpedition.expedition.get_absolute_url}}">{{personexpedition.expedition}}</a>
 </h1>
 
@@ -20,7 +20,7 @@
 {% endfor %}
 </p>
 <p>Status of all wallets for <b>
-<a href="/wallets/person/{{personexpedition.person.first_name|safe}}{{personexpedition.person.last_name|safe}}">{{personexpedition.person}}</a>
+<a href="/wallets/person/{{personexpedition.person.first_name|safe}}{{personexpedition.person.last_name|safe}}">{{personexpedition.person.fullname}}</a>
 </b>
 </p>
 <h3>Table of all trips and surveys aligned by date</h3>
diff --git a/templates/personwallets.html b/templates/personwallets.html
index 6554b3a..f74c231 100644
--- a/templates/personwallets.html
+++ b/templates/personwallets.html
@@ -3,7 +3,7 @@
 {% block title %}One Person Survey scans folders (wallets){% endblock %}
 
 {% block content %}
-<h3>Wallets for <a href="{{person.get_absolute_url}}">{{person}}</a> </h3>
+<h3>Wallets for <a href="{{person.get_absolute_url}}">{{person.fullname}}</a> </h3>
 <p>Each wallet contains the scanned original in-cave survey notes and sketches of 
 plans and elevations. It also contains scans of centre-line survex output on which
 hand-drawn passage sections are drawn. These hand-drawn passages will eventually be 
diff --git a/urls.py b/urls.py
index e1b9d3f..1ae6707 100644
--- a/urls.py
+++ b/urls.py
@@ -124,7 +124,8 @@ trogglepatterns = [
 #   Persons - nasty surname recognition logic fails for 19 people! See also Wallets by person below.
     # path('person/<str:name>', person, name="person"), # This is much more complex than it looks.. 
     re_path(r'^person/(?P<first_name>[A-Z]*[a-z\-\'&;]*)[^a-zA-Z]*(?P<last_name>[a-z\-\']*[^a-zA-Z]*[\-]*[A-Z]*[a-zA-Z\-&;]*)/?', person, name="person"),
-    re_path(r'^personexpedition/(?P<first_name>[A-Z]*[a-z&;]*)[^a-zA-Z]*(?P<last_name>[A-Z]*[a-zA-Z&;]*)/(?P<year>\d+)/?$', personexpedition, name="personexpedition"),
+    #re_path(r'^personexpedition/(?P<first_name>[A-Z]*[a-z&;]*)[^a-zA-Z]*(?P<last_name>[A-Z]*[a-zA-Z&;]*)/(?P<year>\d+)/?$', personexpedition, name="personexpedition"),
+    path('personexpedition/<slug:slug>/<int:year>', personexpedition, name="personexpedition"),
 
 #   Expedition master page & API exports
     re_path(r'^expedition/(\d+)$',  expedition,  name="expedition"),
@@ -210,7 +211,9 @@ trogglepatterns = [
     
 # The data about the wallets themselves, not the scans inside tehm
     path('wallets/year/<int:year>',       walletslistyear,  name="walletslistyear"), # wallets that are for a specific year, as an integer '1985'
-    re_path('wallets/person/(?P<first_name>[A-Z]*[a-z\-\'&;]*)[^a-zA-Z]*(?P<last_name>[a-z\-\']*[^a-zA-Z]*[\-]*[A-Z]*[a-zA-Z\-&;]*)/?',   walletslistperson,  name="walletslistperson"), 
+    # re_path('wallets/person/(?P<first_name>[A-Z]*[a-z\-\'&;]*)[^a-zA-Z]*(?P<last_name>[a-z\-\']*[^a-zA-Z]*[\-]*[A-Z]*[a-zA-Z\-&;]*)/?',   walletslistperson,  name="walletslistperson"), 
+    path('wallets/person/<slug:slug>',   walletslistperson,  name="walletslistperson"), 
+
 
 
 # The tunnel and therion drawings files pageswalletslistcave