diff --git a/core/views/uploads.py b/core/views/uploads.py index d6eb1e4..01d79dc 100644 --- a/core/views/uploads.py +++ b/core/views/uploads.py @@ -40,6 +40,13 @@ todo = """ - parse the uploaded drawing file for links to wallets and scan files as done in parsers/drawings.py + +- move much of the _get() functions in the the _setup() function, esp. the population of the + files and dirs lists for display after a POST. Or remove them where they are not used. + use photoupload() as a the exemplar + +- after refactoring, the commonality between all the file upload forms is now clear. + So we have a job to do in removing the redundancy if we can maintain clarity. - 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() @@ -115,6 +122,7 @@ def expofilerename(request, filepath): Currently this just does files within wallets i.e. in /surveyscans/ and it returns control to the original wallet edit page """ + def is_rotatable(path): """If file is a JPG but has no filename extension, then it must be renamed before it can be rotated. @@ -124,26 +132,20 @@ def expofilerename(request, filepath): return True else: return False - - def rotate_image(): - wallet = str(Path(filepath).parent).lstrip("surveyscans/") + + def rotate_image(ctx): + wallet = str(Path(ctx["filepath"]).parent).lstrip("surveyscans/") cwd = settings.SCANS_ROOT / wallet - print(f"ROTATE \n{cwd=} \n{filename=}") + print(f"ROTATE \n{cwd=} \n{ctx['filename']=}") mogrify = settings.MOGRIFY rot = subprocess.run( - [mogrify, "-rotate", "90", filename], cwd=cwd, capture_output=True, text=True + [mogrify, "-rotate", "90", ctx["filename"]], cwd=cwd, capture_output=True, text=True ) - msgdata = ( - rot.stderr - + "\n" - + rot.stdout - + "\nreturn code: " - + str(rot.returncode) - ) - message = f'! - ROTATE - Success: rotated this file {filename}.' + msgdata + msgdata = rot.stderr + "\n" + rot.stdout + "\nreturn code: " + str(rot.returncode) + message = f'! - ROTATE - Success: rotated this file {ctx["filename"]}.' + msgdata print(message) # DataIssue.objects.create(parser="mogrify", message=message) - + if rot.returncode != 0: msgdata = ( "Ask a nerd to fix this.\n\n" @@ -153,131 +155,165 @@ def expofilerename(request, filepath): + "\n\nreturn code: " + str(rot.returncode) ) - message = ( - f"! - ROTATE - CANNOT blurk for this file {filename}. \n" - + msgdata - ) + message = f"! - ROTATE - CANNOT blurk for this file {ctx['filename']}. \n" + msgdata print(message) DataIssue.objects.create(parser="mogrify", message=message) - return simple_get() - - def simple_get(): + # return to the GET view + return simple_get(ctx) + + def simple_get(ctx): form = ExpofileRenameForm() return render( request, "renameform.html", { "form": form, - "filepath": filepath, - "filename": filename, - "filesize": filesize, - "files": files, - "walletpath": walletpath, - "wallet": wallet, - "notpics": notpics, - "rotatable": rotatable, + "filepath": ctx["filepath"], + "filename": ctx["filename"], + "filesize": ctx["filesize"], + "files": ctx.get("files", []), + "walletpath": ctx["walletpath"], + "wallet": ctx["wallet"], + "notpics": ctx.get("notpics", []), + "rotatable": ctx["rotatable"], }, ) - - if filepath: - # using EXPOFILES not SCANS_ROOT in case we want to extend this to other parts of the system - actualpath = Path(settings.EXPOFILES) / Path(filepath) - else: - message = f'\n File to rename not specified "{filepath}"' - print(message) - return render(request, "errors/generic.html", {"message": message}) - - if not actualpath.is_file(): - message = f'\n File not found when attempting rename "{filepath}"' - print(message) - return render(request, "errors/generic.html", {"message": message}) - else: - filename = Path(filepath).name - walletpath = Path(filepath).parent + + def _setup(filepath_arg): + if not filepath_arg: + message = f'\n File to rename not specified "{filepath_arg}"' + print(message) + return {"response": render(request, "errors/generic.html", {"message": message})} + + actualpath = Path(settings.EXPOFILES) / Path(filepath_arg) + if not actualpath.is_file(): + message = f'\n File not found when attempting rename "{filepath_arg}"' + print(message) + return {"response": render(request, "errors/generic.html", {"message": message})} + + filename = Path(filepath_arg).name + walletpath = Path(filepath_arg).parent wallet = Path(walletpath).name folder = actualpath.parent filesize = f"{actualpath.stat().st_size:,}" - rotatable= is_rotatable(filename) - + rotatable = is_rotatable(filename) - if not actualpath.is_relative_to(Path(settings.SCANS_ROOT)): - message = f'\n Can only do rename within wallets (expofiles/surveyscans/) currently, sorry. "{actualpath}" ' - print(message) - return render(request, "errors/generic.html", {"message": message}) - - files = [] - dirs = [] - notpics =[] - dirpath = actualpath.parent - print(f'! - FORM rename expofile - start \n{filepath=} \n{dirpath=} \n{walletpath=}') - if dirpath.is_dir(): - try: - for f in dirpath.iterdir(): - if f.is_dir(): - for d in f.iterdir(): - dirs.append(f"{f.name}/{d.name}") - if f.is_file(): - if is_rotatable(f.name): # should allow all images here which can be thumsized, not just rotatables. e.g. PDF - files.append(f.name) - else: - notpics.append(f.name) - except FileNotFoundError: - files.append( - "(Error. There should be at least one filename visible here. Try refresh.)" - ) - if request.method == "GET": - return simple_get() - - elif request.method == "POST": + # ensure we are renaming only within wallets (surveyscans) + if not actualpath.is_relative_to(Path(settings.SCANS_ROOT)): + message = f'\n Can only do rename within wallets (expofiles/surveyscans/) currently, sorry. "{actualpath}" ' + print(message) + return {"response": render(request, "errors/generic.html", {"message": message})} + + files = [] + dirs = [] + notpics = [] + dirpath = actualpath.parent + print(f'! - FORM rename expofile - start \n{filepath_arg=} \n{dirpath=} \n{walletpath=}') + + if dirpath.is_dir(): + try: + for f in dirpath.iterdir(): + if f.is_dir(): + for d in f.iterdir(): + dirs.append(f"{f.name}/{d.name}") + if f.is_file(): + if is_rotatable(f.name): + files.append(f.name) + else: + notpics.append(f.name) + except FileNotFoundError: + files.append("(Error. There should be at least one filename visible here. Try refresh.)") + + ctx = { + "filepath": filepath_arg, + "actualpath": actualpath, + "filename": filename, + "walletpath": walletpath, + "wallet": wallet, + "folder": folder, + "filesize": filesize, + "rotatable": rotatable, + "files": sorted(files) if files else [], + "dirs": sorted(dirs) if dirs else [], + "notpics": notpics, + "dirpath": dirpath, + } + return ctx + + def _post(ctx): + # handle POST actions (rotate / rename) form = ExpofileRenameForm(request.POST) if not form.is_valid(): message = f'Invalid form response for file renaming "{request.POST}"' print(message) - return render(request, "errors/generic.html", {"message": message}) + return {"response": render(request, "errors/generic.html", {"message": message})} if "rotate" in request.POST: - return rotate_image() - + # perform rotation and return its response + return rotate_image(ctx) + if "rename" in request.POST: if "renametoname" not in request.POST: print("renametoname not in request.POST") # blank filename passed it, so just treat as another GET - return simple_get() + return simple_get(ctx) - - renameto = sanitize_name(request.POST["renametoname"]) - if (folder / renameto).is_file() or (folder / renameto).is_dir(): + renameto = sanitize_name(request.POST["renametoname"]) + if (ctx["folder"] / renameto).is_file() or (ctx["folder"] / renameto).is_dir(): rename_bad = renameto - message = f'\n Cannot rename to an existing file or folder. "{filename}" -> "{(folder / renameto)}"' + message = f'\n Cannot rename to an existing file or folder. "{ctx["filename"]}" -> "{(ctx["folder"] / renameto)}"' print(message) return render( request, "renameform.html", { "form": form, - "filepath": filepath, - "filename": filename, - "filesize": filesize, - "files": files, - "walletpath": walletpath, - "wallet": wallet, - "notpics": notpics, + "filepath": ctx["filepath"], + "filename": ctx["filename"], + "filesize": ctx["filesize"], + "files": ctx.get("files", []), + "walletpath": ctx["walletpath"], + "wallet": ctx["wallet"], + "notpics": ctx.get("notpics", []), "rename_bad": rename_bad, }, ) - actualpath.rename((folder / renameto)) - message = f'\n RENAMED "{filename}" -> "{(folder / renameto)}"' + # perform rename + ctx["actualpath"].rename((ctx["folder"] / renameto)) + message = f'\n RENAMED "{ctx["filename"]}" -> "{(ctx["folder"] / renameto)}"' print(message) - walletid = actualpath.relative_to(Path(settings.SCANS_ROOT)).parent.stem.replace("#",":") + walletid = ctx["actualpath"].relative_to(Path(settings.SCANS_ROOT)).parent.stem.replace("#", ":") print(walletid) return redirect(f'/survey_scans/{walletid}/') - - else: # not GET or POST - print("UNRECOGNIZED action") - return simple_get() + # fallback: show GET view + return simple_get(ctx) + + # main flow + ctx = _setup(filepath) + if "response" in ctx: + return ctx["response"] + + if request.method == "GET": + return simple_get(ctx) + + if request.method == "POST": + # The refactoring done by VS coe seems to be a bit baroque here, needs further work.. + result = _post(ctx) + # if _post returned a response, return it + if hasattr(result, "status_code") or isinstance(result, dict) and "response" in result: + return result if not isinstance(result, dict) else result["response"] + # else result may be a ctx dict to render + if isinstance(result, dict) and "form" in result and not result["form"].is_valid(): + return simple_get(result) + # if _post returned ctx (or simple_get response), ensure we render current state + if isinstance(result, dict): + return simple_get(result) + + # unrecognized method: default to GET view + return simple_get(ctx) @@ -345,7 +381,6 @@ def photoupload(request, folder=None): ctx["files"] = sorted(files) if files else [] ctx["dirs"] = sorted(dirs) if dirs else [] - print(f"photoupload() _setup ->\n {folder_arg=}\n {dirpath=}\n {urlfile=}\n {urldir=}") return ctx def _post(ctx): @@ -399,12 +434,9 @@ def photoupload(request, folder=None): if (ctx["dirpath"] / saved_filename).is_file(): ctx["actual_saved"].append(saved_filename) ctx["filesaved"] = True - print(ctx) - return ctx def _get(ctx): - return ctx # main flow