Fixing multiple caves with same kataser no

4 files changed, 80 insertions, 23 deletions

diff --git a/core/models_caves.py b/core/models_caves.py
index 2304109..c5d3945 100644
--- a/core/models_caves.py
+++ b/core/models_caves.py
@@ -5,6 +5,7 @@ import logging
 import re
 import json
 from subprocess import call
+from collections import defaultdict
 
 from urllib.parse import urljoin
 
@@ -21,6 +22,7 @@ from django.template import Context, loader
 
 from troggle.core.models import TroggleModel, Person, Expedition
 from troggle.core.models_survex import SurvexStation
+from troggle.core.models import DataIssue
 
 class Area(TroggleModel):
     short_name = models.CharField(max_length=100)
@@ -35,7 +37,7 @@ class Area(TroggleModel):
             return str(self.short_name)
 
     def kat_area(self):
-        if self.short_name in ["1623", "1626"]:
+        if self.short_name in ["1623", "1626", "1624", "1627"]:
             return self.short_name
         elif self.parent:
             return self.parent.kat_area()
@@ -515,30 +517,47 @@ def get_scan_path(instance, filename):
     return os.path.join('./',year,year+r'#'+number,str(instance.contents)+str(instance.number_in_wallet)+r'.jpg')
 
 Gcavelookup = None
+Gcave_count = None
 def GetCaveLookup():
     """lookup function modelled on GetPersonExpeditionNameLookup
     repeated assignment each call, needs refactoring
+    
+    Does NOT detect duplicates! Needs fixing.
+    Needs to be a proper funciton that raises an exception if there is a duplicate.
+    OR we could set it to return None if there are duplictes, and require the caller to 
+    fall back on doing the actual database query it wants rathe rthna using  this cache shortcut
     """
     global Gcavelookup
     if Gcavelookup:
         return Gcavelookup
-    Gcavelookup = {"NONEPLACEHOLDER":None}
+    Gcavelookup = {"NONEPLACEHOLDER": None}
+    global Gcave_count
+    Gcave_count  = defaultdict(int) # sets default value to int(0)
+    
     for cave in Cave.objects.all():
-        Gcavelookup[cave.official_name.lower()] = cave
+        key = cave.official_name.lower()
+        Gcavelookup[key] = cave
+        Gcave_count[key] += 1
         if cave.kataster_number:
-            Gcavelookup[cave.kataster_number] = cave
+            Gcavelookup[cave.kataster_number] = cave # DUPLICATE as we have 1623-55 and 1626-55
+            Gcave_count[cave.kataster_number]  += 1
         if cave.unofficial_number:
             Gcavelookup[cave.unofficial_number.lower()] = cave
+            Gcave_count[cave.unofficial_number.lower()] += 1
         if cave.filename:
             # this is the slug - usually..
             Gcavelookup[cave.filename.replace(".html","").lower()] = cave
+            Gcave_count[cave.filename.replace(".html","").lower()] += 1
         if cave.slug():
             slug = cave.slug()
             Gcavelookup[slug.lower()] = cave
+            Gcave_count[slug.lower()] += 1
     # These are exact matches! edit to check for prefix only!
     # mostly taken from expoweb/noinfo/cave-number-index 
     # and Becka's email of 25 may 2020 on new kataster numbers
     # this should be re-done as a JSON file upload
+    
+    # These might alse create more duplicate entries, so re-write it to check
     Gcavelookup["1987-02"] = Gcavelookup["267"]
     Gcavelookup["1990-01"] = Gcavelookup["171"]
     Gcavelookup["1990-02"] = Gcavelookup["172"]
@@ -624,7 +643,7 @@ def GetCaveLookup():
     Gcavelookup["2016-jb-01"] = Gcavelookup["289"]
     Gcavelookup["2017-pw-01"] = Gcavelookup["277"]
     Gcavelookup["2018-dm-07"] = Gcavelookup["359"]
-    Gcavelookup["2017_cucc_24"] = Gcavelookup["291"]
+    Gcavelookup["2017_cucc_24"] = Gcavelookup["291"] # note _ not - here
     Gcavelookup["2017_cucc_23"] = Gcavelookup["295"]
     Gcavelookup["2017_cucc_28"] = Gcavelookup["290"]
     Gcavelookup["bs17"] = Gcavelookup["283"]
@@ -679,5 +698,12 @@ def GetCaveLookup():
             addmore[id] = Gcavelookup[id].unofficial_number.lower()
     with open("cave-lookup.json", 'w') as f:
         json.dump(addmore, f)
+        
+    for c in Gcave_count:
+        if Gcave_count[c] > 1:
+            message = " ** Duplicate cave id: {}:{}:{}".format(Gcave_count[c], Gcavelookup[c], c)
+            #print(message)
+            #DataIssue.objects.create(parser='caves', message=message)
+            # logdataissues[Gcavelookup[c]]=message # pending troggle-wide issues logging system
 
     return Gcavelookup
diff --git a/core/view_surveys.py b/core/view_surveys.py
index a70fa3d..9b9c533 100644
--- a/core/view_surveys.py
+++ b/core/view_surveys.py
@@ -15,17 +15,20 @@ import parsers.surveys
 '''Some of these views serve files as binary blobs, and simply set the mime type based on the file extension,
 as does the urls.py dispatcher which sends them here. Here they should actually have the filetype checked 
 by looking inside the file before being served.
+
+ need to check if inavlid query string is invalid, or produces multiple replies
+ and render a user-friendly error page.
 '''
 
 def surveyscansfolder(request, path):
     #print [ s.walletname  for s in ScansFolder.objects.all() ]
-    scansfolder = ScansFolder.objects.get(walletname=urlunquote(path))
+    scansfolder = ScansFolder.objects.get(walletname=urlunquote(path))  # need to check if inavlid query string and produce friendly error
     return render(request, 'scansfolder.html', { 'scansfolder':scansfolder, 'settings': settings })
 
 def surveyscansingle(request, path, file):
     '''sends a single binary file to the user,
     '''
-    scansfolder = ScansFolder.objects.get(walletname=urlunquote(path))
+    scansfolder = ScansFolder.objects.get(walletname=urlunquote(path)) # need to check if inavlid query string and produce friendly error
     singlescan = SingleScan.objects.get(scansfolder=scansfolder, name=file)
     # print(" - surveyscansingle {}:{}:{}:".format(path, file, flatviews.getmimetype(file)))
     return HttpResponse(content=open(singlescan.ffile,"rb"), content_type=flatviews.getmimetype(file)) # any type of image
@@ -44,12 +47,12 @@ def tunneldata(request):
 def tunnelfilesingle(request, path):
     '''sends a single binary file to the user, We should have a renderer that syntax-colours this Tunnel xml 
     '''
-    tunnelfile = TunnelFile.objects.get(tunnelpath=urlunquote(path))
+    tunnelfile = TunnelFile.objects.get(tunnelpath=urlunquote(path)) # need to check if inavlid query string and produce friendly error
     tfile = Path(settings.TUNNEL_DATA, tunnelfile.tunnelpath)
     return HttpResponse(content=open(tfile), content_type="text/xhtml") # for display not download
     
 def tunnelfileupload(request, path):
-    tunnelfile = TunnelFile.objects.get(tunnelpath=urlunquote(path))
+    tunnelfile = TunnelFile.objects.get(tunnelpath=urlunquote(path)) # need to check if inavlid query string and produce friendly error
     tfile = Path(settings.TUNNEL_DATA, tunnelfile.tunnelpath)
     
     project, user, password, tunnelversion = request.POST["tunnelproject"], request.POST["tunneluser"], request.POST["tunnelpassword"], request.POST["tunnelversion"]
diff --git a/core/views_caves.py b/core/views_caves.py
index 95de183..946eba9 100644
--- a/core/views_caves.py
+++ b/core/views_caves.py
@@ -11,11 +11,12 @@ from django.conf import settings
 from django.urls import reverse
 from django.http import HttpResponse, HttpResponseRedirect
 from django.shortcuts import get_object_or_404, render
+from django.core.exceptions import ObjectDoesNotExist, MultipleObjectsReturned
 
 import troggle.settings as settings
-import troggle.core.models as models
-from troggle.core.models import Expedition
-from troggle.core.models_caves import CaveSlug, Cave, CaveAndEntrance, QM, EntranceSlug, Entrance, Area, SurvexStation
+#import troggle.core.models as models
+from troggle.core.models import Expedition, DataIssue
+from troggle.core.models_caves import CaveSlug, Cave, CaveAndEntrance, QM, EntranceSlug, Entrance, Area, SurvexStation, GetCaveLookup
 from troggle.core.forms import CaveForm, CaveAndEntranceFormSet, VersionControlCommentForm, EntranceForm, EntranceLetterForm
 from troggle.helper import login_required_if_public
 
@@ -41,14 +42,14 @@ class MapLocations(object):
                     k = ent.caveandentrance_set.all()[0].cave
                 except:
                     message = " ! Failed to get Cave linked to Entrance:{} from:{} best:{}".format(ent.name, ent.filename, ent.best_station())
-                    models.DataIssue.objects.create(parser='entrances', message=message)
+                    DataIssue.objects.create(parser='entrances', message=message)
                     print(message)
                     raise
                 try:
                     areaName = k.getArea().short_name
                 except:
                     message = " ! Failed to get Area on cave '{}' linked to Entrance:{} from:{} best:{}".format(cave, ent.name, ent.filename, ent.best_station())
-                    models.DataIssue.objects.create(parser='entrances', message=message)
+                    DataIssue.objects.create(parser='entrances', message=message)
                     print(message)
                     raise
                 self.p.append((ent.best_station(), "%s-%s" % (areaName, str(ent)[5:]), ent.needs_surface_work(), str(ent)))
@@ -58,13 +59,23 @@ class MapLocations(object):
         return "{} map locations".format(len(self.p))
 
 def getCave(cave_id):
-    """Returns a cave object when given a cave name or number. It is used by views including cavehref, ent, and qm."""
-    # REPLACE OR MERGE this with other functions that do exactly th same thing!
+    '''Returns a cave object when given a cave name or number. It is used by views including cavehref, ent, and qm.
+    
+    TO DO: search GCavelookup first, which should raise a MultpleObjectsReturned exception if there
+    are duplicates'''
     try:
         cave = Cave.objects.get(kataster_number=cave_id)
+    except Cave.MultipleObjectsReturned as ex:
+        raise MultipleObjectsReturned("Duplicate kataster number")  from ex # propagate this up
+
     except Cave.DoesNotExist:
-        cave = Cave.objects.get(unofficial_number=cave_id)
-    return cave
+        Gcavelookup = GetCaveLookup() # dictionary makes strings to Cave objects
+        if cave_id in Gcavelookup:
+            return Gcavelookup[cave_id] 
+        else: 
+             raise ObjectDoesNotExist("No cave found with this identifier in any id field")
+    except:
+        raise ObjectDoesNotExist("No cave found with this identifier in any id field")
 
 def pad5(x):
    return "0" * (5 -len(x.group(0))) + x.group(0) 
@@ -75,7 +86,7 @@ def numericalcmp(x, y):
 
 def caveKey(x):
     """python3 function for sort. Done in a hurry.
-    Note that cave kataster numbers are not generally integers.
+    Note that cave kataster numbers are not always integers.
     This needs to be fixed make a decent sort order.
     """ 
     if not x.kataster_number:
@@ -107,7 +118,12 @@ def caveindex(request):
     return render(request,'caveindex.html', {'caves1623': caves1623, 'caves1626': caves1626, 'notablecaves':getnotablecaves(), 'cavepage': True})
 
 def cave3d(request, cave_id=''):
-    cave = getCave(cave_id)
+    try:
+        cave = getCave(cave_id)
+    except Cave.MultipleObjectsReturned: # entirely the wrong action, REPLACE with the right display
+        caves = Cave.objects.filter(kataster_number=cave_id) 
+        return render(request, 'svxcaveseveral.html', {'settings': settings, "caves":caves })
+    
     survexfilename = settings.SURVEX_DATA + cave.survex_file
     threedfilename = settings.THREEDCACHEDIR + '%s.3d' % cave_id
     if True or os.path.getmtime(survexfilename) > os.path.getmtime(threedfilename):
@@ -121,9 +137,16 @@ def cave3d(request, cave_id=''):
     return response
 
 def cave(request, cave_id='', offical_name=''):
-    cave=getCave(cave_id)
+    try:
+        cave=getCave(cave_id) 
+    except MultipleObjectsReturned:
+        caves = Cave.objects.filter(kataster_number=cave_id) 
+        return render(request, 'svxcaveseveral.html', {'settings': settings, "caves":caves }) # not the right template, needs a specific one
+    except ObjectDoesNotExist:
+        return render(request, 'svxcavesingle404.html', {'settings': settings, "cave":cave_id })
+
     if cave.non_public and settings.PUBLIC_SITE and not request.user.is_authenticated():
-        return render(request,'nonpublic.html', {'instance': cave, 'cavepage': True, 'cave_id': cave_id})
+        return render(request, 'nonpublic.html', {'instance': cave, 'cavepage': True, 'cave_id': cave_id})
     else:
         return render(request,'cave.html', {'settings': settings, 'cave': cave, 'cavepage': True, 'cave_id': cave_id})
 
@@ -254,8 +277,11 @@ def editEntrance(request, caveslug, slug=None):
 def qm(request,cave_id,qm_id,year,grade=None):
     year=int(year)
     try:
-        qm=getCave(cave_id).get_QMs().get(number=qm_id,found_by__date__year=year)
+        qm=getCave(cave_id).get_QMs().get(number=qm_id,found_by__date__year=year) 
         return render(request,'qm.html',locals())
+    except Cave.MultipleObjectsReturned: # entirely the wrong action, REPLACE with the right display
+        caves = Cave.objects.filter(kataster_number=cave_id) 
+        return render(request, 'svxcaveseveral.html', {'settings': settings, "caves":caves })
 
     except QM.DoesNotExist:
         url=urllib.parse.urljoin(settings.URL_ROOT, r'/admin/core/qm/add/'+'?'+  r'number=' + qm_id)
diff --git a/core/views_survex.py b/core/views_survex.py
index ef554aa..87f4430 100644
--- a/core/views_survex.py
+++ b/core/views_survex.py
@@ -377,6 +377,8 @@ def survexcavesingle(request, survex_cave):
     '''parsing all the survex files of a single cave and showing that it's consistent and can find all 
     the files and people.   Should explicity fix the kataster number thing.
     kataster numbers are not unique across areas. Fix this.
+    
+    Should use getCave() from models_caves
     '''
     sc = survex_cave
     try: