diff --git a/core/views/uploads.py b/core/views/uploads.py index 9e4f26c..4afb372 100644 --- a/core/views/uploads.py +++ b/core/views/uploads.py @@ -676,7 +676,11 @@ def dwgupload(request, folder=None, gitdisable="no"): AND registers it into the :drawings: git repo. This does NOT use a Django model linked to a Django form. Just a simple Django form. - You will find the Django documentation on forms very confusing, This is simpler. + You will find the Django documentation on class-based forms very confusing, This is simpler. + + This does not even use a Django View, it is entirely built on functions not classes. + See https://spookylukey.github.io/django-views-the-right-way/index.html + but with an even greater emphasis on simple visibility for nn-Django programmers. We could validate the uploaded files as being a valid files using an XML parser, not a dubious script or hack, but this won't work on Tunnel files as Tunnel does not produce exactly valid xml (!) @@ -685,12 +689,17 @@ def dwgupload(request, folder=None, gitdisable="no"): several times in one session, and expects them to be overwritten in the database. (Although the actual file will be duplicated in the filesystem with different random name ending, and this will need to be cleaned-up manually by a nerd later.) + + instructions to copilot: + 'refactor dwgupload() to reduce the size of the large if statement using new + functions _setup(), _get() and post()' + Stunningly, this was enough to do the whole job. It did remove some comments though, + which have been manually reinstated. + Refactored to use _setup, _post and _get helpers to reduce a large if/else block. """ def dwgvalid(name): - if name in [ - ".gitignore", - ]: + if name in [".gitignore"]: return False if Path(name).suffix.lower() in [".xml", ".th", ".th2", "", ".svg", ".txt"]: return True # dangerous, we should check the actual file binary signature @@ -703,164 +712,156 @@ def dwgupload(request, folder=None, gitdisable="no"): ]: return False if Path(name).suffix.lower() in [ - ".xml", - ".th", - ".th2", - "", - ".svg", - ".txt", - ".jpg", - ".jpeg", - ".png", - ".pdf", - ".top", - ".topo", + ".xml", ".th", ".th2", "", ".svg", ".txt", ".jpg", ".jpeg", ".png", ".pdf", ".top", ".topo", ]: return True # dangerous, we should check the actual file binary signature return False - filesaved = False - actual_saved = [] - refused = [] - doesnotexist = "" - # print(f'! - FORM dwgupload - start "{folder}" - gitdisable "{gitdisable}"') - if folder is None: - folder = "" # improve this later - dirpath = Path(settings.DRAWINGS_DATA) - urlfile = "/dwgdataraw" - urldir = "/dwgupload" - else: - dirpath = Path(settings.DRAWINGS_DATA, folder) - urlfile = Path("/dwgdataraw/") / folder - urldir = Path("/dwgupload/") / folder + def _setup(folder_arg): + # initialize common state and return context dict + ctx = {} + if folder_arg is None: + folder_arg = "" + dirpath = Path(settings.DRAWINGS_DATA) + urlfile = "/dwgdataraw" + urldir = "/dwgupload" + else: + dirpath = Path(settings.DRAWINGS_DATA, folder_arg) + urlfile = Path("/dwgdataraw/") / folder_arg + urldir = Path("/dwgupload/") / folder_arg - identified_login = is_identified_user(request.user) - editor = get_editor(request) - form = DrawingsFilesForm() + ctx.update( + { + "folder": folder_arg, + "dirpath": dirpath, + "urlfile": urlfile, + "urldir": urldir, + "identified_login": is_identified_user(request.user), + "editor": get_editor(request), + "form": DrawingsFilesForm(), + "filesaved": False, + "actual_saved": [], + "refused": [], + "doesnotexist": "", + } + ) + return ctx - if request.method == "POST": + def _post(ctx): + # handle POST -- validate form and save files, update ctx in-place form = DrawingsFilesForm(request.POST, request.FILES) - if form.is_valid(): - # print(f'! - FORM dwgupload - POST valid: "{request.FILES["uploadfiles"]}" ') - editor = form.cleaned_data["who_are_you"] - editor = git_string(editor) + ctx["form"] = form + if not form.is_valid(): + return ctx # will be rendered like GET - f = request.FILES["uploadfiles"] - multiple = request.FILES.getlist("uploadfiles") - savepath = Path(settings.DRAWINGS_DATA, folder) - fs = FileSystemStorage(savepath) + editor = form.cleaned_data["who_are_you"] + editor = git_string(editor) + ctx["editor"] = editor - actual_saved = [] - refused = [] + multiple = request.FILES.getlist("uploadfiles") + savepath = Path(settings.DRAWINGS_DATA, ctx["folder"]) + fs = FileSystemStorage(savepath) + ctx["actual_saved"] = [] + ctx["refused"] = [] - # GIT see also core/views/expo.py editexpopage() - # GIT see also core/models/cave.py writetrogglefile() - if gitdisable != "yes": # set in url 'dwguploadnogit/' - git = settings.GIT - else: - git = "echo" - # print(f'git DISABLED {f.name}') + git = settings.GIT if gitdisable != "yes" else "echo" + commands = [] - # For saving, and then comitting, multiple files, we should be using write_and_commit() - # - # try: - # write_and_commit([(filepath, newtext, "utf-8")], f"Online edit of {path}", editor) - # except WriteAndCommitError as e: - # return render(request, "errors/generic.html", {"message": e.message}) - if multiple: - for f in multiple: - # print(f'! - FORM dwgupload - file {f} in {multiple=}') - if dwgvalid(f.name): - try: # crashes in Django os.chmod call if on WSL without metadata drvfs, but does save file! - saved_filename = fs.save(f.name, content=f) - except: - print( - f'! - FORM dwgupload - \n!! Permissions failure ?! on attempting to save file "{f.name}" in "{savepath}". Attempting to continue..' - ) - if "saved_filename" in locals(): - filepath = dirpath / saved_filename - if filepath.is_file(): - actual_saved.append(saved_filename) - - if gitdisable != "yes": - commands = git_add(filepath, dirpath) - - dwgfile, created = DrawingFile.objects.get_or_create( - dwgpath=saved_filename, dwgname=Path(f.name).stem, filesize=f.size - ) - dwgfile.save() - else: - message = f"! - FORM dwgupload - NOT A FILE {Path(dirpath, saved_filename)=}. " - print(message) - else: - message = f"! - FORM dwgupload - Save failure for {f.name}. Changes NOT saved." - print(message) - return render(request, "errors/generic.html", {"message": message}) + if multiple: + for f in multiple: + if not dwgvalid(f.name): + ctx["refused"].append(f.name) + continue + try: + saved_filename = fs.save(f.name, content=f) + except Exception as e: + # save failed: abort with error page + message = f'! - FORM dwgupload - Permissions failure saving "{f.name}" in "{savepath}": {e}' + print(message) + return render(request, "errors/generic.html", {"message": message}) - if saved_filename != f.name: - # message = f'! - FORM dwgupload - Save RENAME {f.name} renamed as {saved_filename}. This is OK.' - # print(message) - pass - - else: - refused.append(f.name) - # print(f'REFUSED {f.name}') - - if actual_saved: - filesaved = True - if len(actual_saved) > 1: - dots = f"{len(actual_saved)} files" + filepath = ctx["dirpath"] / saved_filename + if filepath.is_file(): + ctx["actual_saved"].append(saved_filename) + if gitdisable != "yes": + commands = git_add(filepath, ctx["dirpath"]) + dwgfile, created = DrawingFile.objects.get_or_create( + dwgpath=saved_filename, dwgname=Path(f.name).stem, filesize=f.size + ) + dwgfile.save() else: - dots = f"{actual_saved[0]}" - commit_msg = f"Drawings upload - {dots}" - if gitdisable != "yes": - git_commit(dirpath, commit_msg, editor, commands) - else: # maybe all were refused by the suffix test in dwgvalid() - message = f"! - FORM dwgupload - Nothing actually saved. All were refused. {actual_saved=}" + message = f"! - FORM dwgupload - NOT A FILE {Path(ctx['dirpath'], saved_filename)=}." + print(message) + + if ctx["actual_saved"]: + ctx["filesaved"] = True + if len(ctx["actual_saved"]) > 1: + dots = f"{len(ctx['actual_saved'])} files" + else: + dots = f"{ctx['actual_saved'][0]}" + commit_msg = f"Drawings upload - {dots}" + if gitdisable != "yes": + git_commit(ctx["dirpath"], commit_msg, editor, commands) + else: + if ctx["refused"]: + message = f"! - FORM dwgupload - Nothing actually saved. All were refused. {ctx['actual_saved']=}" print(message) - # GET request starts here, also drops through from POST - files = [] - dirs = [] - # print(f'! - FORM dwgupload - start {folder=} \n"{dirpath=}" \n"{dirpath.parent=}" \n"{dirpath.exists()=}"') - try: - for f in dirpath.iterdir(): - if f.is_dir(): - if f.name not in [".git"]: - dirs.append(f.name) - continue - if f.is_file(): - if dwgvaliddisp(f.name): - files.append(f.name) - continue - except FileNotFoundError: - doesnotexist = True - if files: - files = sorted(files) + return ctx + + def _get(ctx): + # build file/directory listings and prepare form readonly state if needed + files = [] + dirs = [] + try: + for f in ctx["dirpath"].iterdir(): + if f.is_dir(): + if f.name not in [".git"]: + dirs.append(f.name) + continue + if f.is_file(): + if dwgvaliddisp(f.name): + files.append(f.name) + continue + except FileNotFoundError: + ctx["doesnotexist"] = True + + ctx["files"] = sorted(files) if files else [] + ctx["dirs"] = sorted(dirs) if dirs else [] + + if ctx["identified_login"]: + ctx["form"].fields["who_are_you"].widget.attrs["readonly"] = "readonly" + + return ctx + + # main flow: setup, then POST or GET handlers, then render + ctx = _setup(folder) + + if request.method == "POST": + ctx = _post(ctx) + # if form invalid, still show GET 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) - if dirs: - dirs = sorted(dirs) - - if identified_login: - # disable editing the git id string as we get it from the logged-on user data - form.fields["who_are_you"].widget.attrs["readonly"]="readonly" response = render( request, - "dwguploadform.html", # a bit more primitive than many forms in troggle, everything is very explicit and doesn't use widgets + "dwguploadform.html", { - "form": form, - "identified_login": identified_login, - "doesnotexist": doesnotexist, - "urlfile": urlfile, - "urldir": urldir, - "folder": folder, - "files": files, - "dirs": dirs, - "filesaved": filesaved, - "actual_saved": actual_saved, - "refused": refused, - "who_are_you": editor, + "form": ctx["form"], + "identified_login": ctx["identified_login"], + "doesnotexist": ctx.get("doesnotexist", ""), + "urlfile": ctx["urlfile"], + "urldir": ctx["urldir"], + "folder": ctx["folder"], + "files": ctx.get("files", []), + "dirs": ctx.get("dirs", []), + "filesaved": ctx.get("filesaved", False), + "actual_saved": ctx.get("actual_saved", []), + "refused": ctx.get("refused", []), + "who_are_you": ctx["editor"], }, ) - response.set_cookie('editor_id', editor, max_age=get_cookie_max_age(request)) # cookie expires after get_cookie_max_age(request) seconds - return response + response.set_cookie("editor_id", ctx["editor"], max_age=get_cookie_max_age(request)) + return response \ No newline at end of file