refactored photoupload, but a small bug

core/views/uploads.py

@@ -38,6 +38,9 @@ todo = """
 - Register the uploaded drawing file (views/uploads.py) using the functions in parsers/drawings.py so that queries of the database
   can find newly uploaded files without having to do a database reset.
 
+- parse the uploaded drawing file for links to wallets and scan files as done 
+  in parsers/drawings.py
+
 - Ideally we should validate uploaded file as being a valid file type, not a dubious script or hack
   Validate image files using a magic recogniser in walletedit()
   https://pypi.org/project/reportlab/   or 
@@ -45,28 +48,19 @@ todo = """
   
 - Validate Tunnel & Therion files using an XML parser in dwgupload(). Though Julian says 
   tunnel is  only mostly correct XML, and it does fail at least one XML parser. 
-  Many tunnel files have non-ascii bytes in them!
+  Many tunnel files have non-ascii bytes in them, but they should all be utf-8.
   
-- parse the uploaded drawing file for links to wallets and scan files as done 
-  in parsers/drawings.py
 
 - Enable folder creation in dwguploads or as a separate form
 
 - Enable file rename on expofiles, not just for /surveyscans/ (aka wallets)
 
 - Make file rename utility less ugly.
+
+- refactor the WalletFilesForm and PhotoUpload form to use the _setup, _get, _post idiom.
 """
 
 
-class DrawingsFilesForm(forms.Form):  # not a model-form, just a form-form
-    uploadfiles = forms.FileField()
-    identified_login = forms.BooleanField(required=False,widget=forms.CheckboxInput(attrs={"onclick":"return false"})) # makes it readonly
-    who_are_you = forms.CharField(
-        widget=forms.TextInput(
-            attrs={"size":  100, "placeholder": "You are editing this page,  who are you ? e.g. 'Becka' or 'Animal <mta@gasthof.expo>'", 
-            "style": "vertical-align: text-top;"}
-        )
-    )
 
 class WalletFilesForm(forms.Form):  # not a model-form, just a form-form
     """Used only for uploading to expofiles/surveyscans/<year>/<wallet>
@@ -91,6 +85,15 @@ class GPXfixForm(forms.Form):  # not a model-form, just a form-form
     station = forms.CharField(strip=True)
     uploadfiles = forms.FileField()
 
+class DrawingsFilesForm(forms.Form):  # not a model-form, just a form-form
+    uploadfiles = forms.FileField()
+    identified_login = forms.BooleanField(required=False,widget=forms.CheckboxInput(attrs={"onclick":"return false"})) # makes it readonly
+    who_are_you = forms.CharField(
+        widget=forms.TextInput(
+            attrs={"size":  100, "placeholder": "You are editing this page,  who are you ? e.g. 'Becka' or 'Animal <mta@gasthof.expo>'", 
+            "style": "vertical-align: text-top;"}
+        )
+    )
     
 class FilesRenameForm(forms.Form):  # not a model-form, just a form-form
     """Used only for renaming photos in /expofiles/photos/
@@ -275,6 +278,9 @@ def expofilerename(request, filepath):
         print("UNRECOGNIZED action")
         return simple_get()
 
+
+
+
 @login_required_if_public
 def photoupload(request, folder=None):
     """Upload photo image files into /expofiles/photos/<year>/<photographer>/
@@ -290,266 +296,161 @@ def photoupload(request, folder=None):
     
     Pending generic file renaming capability more generally.
     """
-    year = current_expo()
-    # year = settings.PHOTOS_YEAR
-    filesaved = False
-    actual_saved = []
 
-    context = {"year": year, "placeholder": "AnathemaDevice"}
+    def _setup(folder_arg):
+        year = current_expo()
+        yearpath = Path(settings.PHOTOS_ROOT, year)
+        # merge previous 'context' fields into ctx
+        ctx = {
+            "year": year,
+            "placeholder": "AnathemaDevice",
+            "filesaved": False,
+            "actual_saved": [],
+            "yearpath": yearpath,
+            "form": FilesRenameForm(),
+            "formd": PhotographerForm(),
+        }
 
-    yearpath = Path(settings.PHOTOS_ROOT, year)
+        # Normalize folder and derive dirpath/urlfile/urldir like original logic
+        if folder_arg == str(year) or folder_arg == str(year) + "/":
+            folder_arg = None
 
-    if folder == str(year) or folder == str(year) + "/":
+        if folder_arg is None:
-        folder = None
+            folder_arg = ""  # improve this later
-
-    if folder is None:
-        folder = ""  # improve this later
-        dirpath = Path(settings.PHOTOS_ROOT, year)
-        urlfile = f"/expofiles/photos/{year}"
-        urldir = f"/photoupload/{year}"
-    else:  # it will contain the year as well as the photographer
-        dirpath = Path(settings.PHOTOS_ROOT, folder)
-        if dirpath.is_dir():
-            urlfile = f"/expofiles/photos/{folder}"
-            urldir = Path("/photoupload") / folder
-        else:
-            folder = ""  # improve this later
             dirpath = Path(settings.PHOTOS_ROOT, year)
             urlfile = f"/expofiles/photos/{year}"
             urldir = f"/photoupload/{year}"
+        else:  # it will contain the year as well as the photographer
+            dirpath = Path(settings.PHOTOS_ROOT, folder_arg)
+            if dirpath.is_dir():
+                urlfile = f"/expofiles/photos/{folder_arg}"
+                urldir = Path("/photoupload") / folder_arg
+            else:
+                folder_arg = ""  # improve this later
+                dirpath = Path(settings.PHOTOS_ROOT, year)
+                urlfile = f"/expofiles/photos/{year}"
+                urldir = f"/photoupload/{year}"
 
-    form = FilesRenameForm()
+        ctx.update({"folder": folder_arg, "dirpath": dirpath, "urlfile": urlfile, "urldir": urldir})
-    formd = PhotographerForm()
+        print(f"photoupload() _setup -> {folder_arg=} {dirpath=} {urlfile=} {urldir=}")
+        return ctx
 
-    if request.method == "POST":
+    def _post(ctx):
+        # keep original POST behavior and prints
         if "photographer" in request.POST:
             # then we are creating a new folder
             formd = PhotographerForm(request.POST)
+            ctx["formd"] = formd
             if formd.is_valid():
                 newphotographer = sanitize_name(request.POST["photographer"])
                 try:
-                    (yearpath / newphotographer).mkdir(parents=True, exist_ok=True)
+                    (ctx["yearpath"] / newphotographer).mkdir(parents=True, exist_ok=True)
-                except:
+                except Exception:
-                    message = f'\n !! Permissions failure ?! 0 attempting to mkdir "{(yearpath / newphotographer)}"'
+                    message = f'\n !! Permissions failure ?! 0 attempting to mkdir "{(ctx["yearpath"] / newphotographer)}"'
                     print(message)
                     return render(request, "errors/generic.html", {"message": message})
+            return ctx
 
-        else:
+        # else branch: handle uploads / renames
-            # then we are renaming the file ?
+        form = FilesRenameForm(request.POST, request.FILES)
-            form = FilesRenameForm(request.POST, request.FILES)
+        ctx["form"] = form
-            if form.is_valid():
+        if not form.is_valid():
-                f = request.FILES["uploadfiles"]
+            return ctx
-                multiple = request.FILES.getlist("uploadfiles")
-                # NO CHECK that the files being uploaded are image files
-                fs = FileSystemStorage(dirpath)
 
-                renameto = sanitize_name(request.POST["renameto"])
+        # original behaviour: support renameto and multiple files
+        f_single = request.FILES.get("uploadfiles")
+        multiple = request.FILES.getlist("uploadfiles")
+        fs = FileSystemStorage(ctx["dirpath"])
 
-                actual_saved = []
+        renameto = sanitize_name(request.POST.get("renameto", ""))
-                if multiple:
+        ctx["actual_saved"] = []
-                    if len(multiple) == 1:
+        ctx["filesaved"] = False
-                        if renameto != "":
-                            try:  # crashes in Django os.chmod call if on WSL, but does save file!
-                                saved_filename = fs.save(renameto, content=f)
-                            except:
-                                print(
-                                    f'\n !! Permissions failure ?! 1 attempting to save "{f.name}" in "{dirpath}" {renameto=}'
-                                )
-                                if "saved_filename" in locals():
-                                    if saved_filename.is_file():
-                                        actual_saved.append(saved_filename)
-                                        filesaved = True
-                        else:  # multiple is the uploaded content
-                            try:  # crashes in Django os.chmod call if on WSL, but does save file!
-                                saved_filename = fs.save(f.name, content=f)
-                            except:
-                                print(
-                                    f'\n !! Permissions failure ?! 2 attempting to save "{f.name}" in "{dirpath}" {renameto=}'
-                                )
-                                if "saved_filename" in locals():
-                                    if saved_filename.is_file():
-                                        actual_saved.append(saved_filename)
-                                        filesaved = True
-                    else:  # multiple is a list of content
-                        for f in multiple:
-                            try:  # crashes in Django os.chmod call if on WSL, but does save file!
-                                saved_filename = fs.save(f.name, content=f)
-                            except:
-                                print(
-                                    f'\n !! Permissions failure ?! 3 attempting to save "{f.name}" in "{dirpath}" {renameto=}'
-                                )
-                                if "saved_filename" in locals():
-                                    if saved_filename.is_file():
-                                        actual_saved.append(saved_filename)
-                                        filesaved = True
-    files = []
-    dirs = []
-    try:
-        for f in dirpath.iterdir():
-            if f.is_dir():
-                dirs.append(f.name)
-            if f.is_file():
-                files.append(f.name)
-    except FileNotFoundError:
-        files.append("(no folder yet - would be created)")
-    if len(files) > 0:
-        files = sorted(files)
 
-    if dirs:
+        # If multiple uploaded files
-        dirs = sorted(dirs)
+        if multiple:
+            if len(multiple) == 1:
+                # single-file upload, possibly renamed
+                f = multiple[0]
+                filename_to_save = renameto if renameto != "" else f.name
+                try:
+                    saved_filename = fs.save(filename_to_save, content=f)
+                except Exception:
+                    print(f'\n !! Permissions failure ?! 1 attempting to save "{f.name}" in "{ctx["dirpath"]}" {renameto=}')
+                    if "saved_filename" in locals():
+                        if (ctx["dirpath"] / saved_filename).is_file():
+                            ctx["actual_saved"].append(saved_filename)
+                            ctx["filesaved"] = True
+                else:
+                    if (ctx["dirpath"] / saved_filename).is_file():
+                        ctx["actual_saved"].append(saved_filename)
+                        ctx["filesaved"] = True
+            else:
+                # multiple files, ignore renameto and save each
+                for f in multiple:
+                    try:
+                        saved_filename = fs.save(f.name, content=f)
+                    except Exception:
+                        print(f'\n !! Permissions failure ?! 3 attempting to save "{f.name}" in "{ctx["dirpath"]}" {renameto=}')
+                        if "saved_filename" in locals():
+                            if (ctx["dirpath"] / saved_filename).is_file():
+                                ctx["actual_saved"].append(saved_filename)
+                                ctx["filesaved"] = True
+                        continue
+                    if (ctx["dirpath"] / saved_filename).is_file():
+                        ctx["actual_saved"].append(saved_filename)
+                        ctx["filesaved"] = True
+
+        return ctx
+
+    def _get(ctx):
+        files = []
+        dirs = []
+        try:
+            for f in ctx["dirpath"].iterdir():
+                if f.is_dir():
+                    dirs.append(f.name)
+                if f.is_file():
+                    files.append(f.name)
+        except FileNotFoundError:
+            files.append("(no folder yet - would be created)")
+        if len(files) > 0:
+            files = sorted(files)
+        if dirs:
+            dirs = sorted(dirs)
+        ctx["files"] = files
+        ctx["dirs"] = dirs
+        return ctx
+
+    # main flow
+    ctx = _setup(folder)
+
+    if request.method == "POST":
+        ctx = _post(ctx)
+        # if form invalid, still show GET-like view (ctx includes form with errors)
+        if isinstance(ctx, dict) and "form" in ctx and not ctx["form"].is_valid():
+            ctx = _get(ctx)
+    else:
+        ctx = _get(ctx)
 
     return render(
         request,
         "photouploadform.html",
         {
-            "form": form,
+            "form": ctx.get("form", FilesRenameForm()),
-            **context,
+            "year": ctx["year"],
-            "urlfile": urlfile,
+            "placeholder": ctx["placeholder"],
-            "urldir": urldir,
+            "urlfile": ctx["urlfile"],
-            "folder": folder,
+            "urldir": ctx["urldir"],
-            "files": files,
+            "folder": ctx["folder"],
-            "dirs": dirs,
+            "files": ctx.get("files", []),
-            "filesaved": filesaved,
+            "dirs": ctx.get("dirs", []),
-            "actual_saved": actual_saved,
+            "filesaved": ctx.get("filesaved", False),
+            "actual_saved": ctx.get("actual_saved", []),
         },
     )
 
-@login_required_if_public
-def gpxupload(request, folder=None):
-    """Copy of photo upload
-    folder is the "path"
-    """
-    def gpxvalid(name):
-        # dangerous, we should check the actual file binary signature
-        return Path(name).suffix.lower() in [".xml", ".gpx"]
 
 
-    print(f"gpxupload() {folder=}")
-    year = current_expo()
-    filesaved = False
-    actual_saved = []
-
-    context = {"year": year, "placeholder": "AnathemaDevice"}
-
-    yearpath = Path(settings.EXPOFILES) / "gpslogs" / year
-
-    if folder == str(year) or folder == str(year) + "/":
-        folder = None
-
-    if folder is None:
-        folder = ""  # improve this later
-        dirpath = yearpath
-        urlfile = f"/expofiles/gpslogs/{year}"
-        urldir = f"/gpxupload/{year}"
-    else:  # it will contain the year as well as the prospector
-        dirpath = Path(settings.EXPOFILES) / "gpslogs" / folder
-        if dirpath.is_dir():
-            urlfile = f"/expofiles/gpslogs/{folder}"
-            urldir = Path("/gpxupload") / folder
-        else:
-            folder = ""  # improve this later
-            dirpath = yearpath
-            urlfile = f"/expofiles/gpslogs/{year}"
-            urldir = f"/gpxupload/{year}"
-
-    print(f"gpxupload() {folder=} {dirpath=} {urlfile=} {urldir=}")
-
-    formd = GPXuploadForm()
-    print(f"gpxupload() {form=} {formd=} ")
-    
-
-    if request.method == "POST":
-        print(f"gpxupload() method=POST")
-        for i in request.POST:
-            print("   ",i)
-
-        if "prospector" in request.POST:
-            print(f"gpxupload() {request.POST=}\n  {request.POST['prospector']=}")
-            formd = GPXuploadForm(request.POST)
-            if formd.is_valid():
-                newprospector = sanitize_name(request.POST["prospector"])
-                print(f"gpxupload() {newprospector=}")
-                try:
-                    (yearpath / newprospector).mkdir(parents=True, exist_ok=True)
-                except Exception as e:
-                    message = f'\n !! Permissions failure ?! 0 attempting to mkdir "{(yearpath / newprospector)}": {e}'
-                    print(message)
-                    raise
-                    return render(request, "errors/generic.html", {"message": message})
-
-        else:
-            print(f"gpxupload() no prospector field")
-            print(f"gpxupload() {request.FILES=}")
-            for i in request.FILES:
-                print("   ",i)
-
-            if True:
-                print(f"gpxupload() about to look at request.FILES")
-                f = request.FILES["uploadfiles"]
-                multiple = request.FILES.getlist("uploadfiles")
-                # NO CHECK that the files being uploaded are image files
-                fs = FileSystemStorage(dirpath)
-
-
-                actual_saved = []
-                if multiple:
-
-                    for f in multiple:                            
-                        if gpxvalid(f.name):
-                            try:  # crashes in Django os.chmod call if on WSL, but does save file!
-                                saved_filename = fs.save(f.name, content=f)
-                            except:
-                                print(
-                                    f'\n !! Permissions failure ?! 3 attempting to save "{f.name}" in "{dirpath}" {renameto=}'
-                                )
-                                if "saved_filename" in locals():
-                                    if saved_filename.is_file():
-                                        actual_saved.append(saved_filename)
-                                        filesaved = True
-                        else:
-                            print(f"gpxupload(): not a GPX file {f.name=}")
-                                
-    print(f"gpxupload() drop through")
-    files = []
-    dirs = []
-    try:
-        for f in dirpath.iterdir():
-            if f.is_dir():
-                dirs.append(f.name)
-            if f.is_file():
-                files.append(f.name)                
-    except FileNotFoundError:
-        files.append("(no folder yet - would be created)")
-    except Exception as e:
-        print(f"gpxupload() EXCEPTION\n {e}")
-    if len(files) > 0:
-        files = sorted(files)
-
-    if dirs:
-        dirs = sorted(dirs)
-
-    print(f"gpxupload() about to render..")
-    return render(
-        request,
-        "gpxuploadform.html",
-        {
-            "form": form,
-            **context,
-            "urlfile": urlfile,
-            "urldir": urldir,
-            "folder": folder,
-            "files": files,
-            "dirs": dirs,
-            "filesaved": filesaved,
-            "actual_saved": actual_saved,
-        },
-    )
-
-def analyse_gpx(saved_filename, content):
-    """For an uploaded GPX file, analyse it to get a *fix number
-    """
-    print(f"analyse_gpx(): {saved_filename} -- {content.name} length: {len(content)} bytes")
-
 @login_required_if_public
 def gpxupload(request, folder=None):
     """Copy of photo upload folder is the "path"
@@ -711,6 +612,10 @@ def gpxupload(request, folder=None):
         },
     )
 
+def analyse_gpx(saved_filename, content):
+    """For an uploaded GPX file, analyse it to get a *fix number
+    """
+    print(f"analyse_gpx(): {saved_filename} -- {content.name} length: {len(content)} bytes")
 
 @login_required_if_public
 def gpxfix(request):