nearly done cave edit commit thing with cookie

3 files changed, 164 insertions, 110 deletions

diff --git a/core/utils.py b/core/utils.py
index 798e48f..904faad 100644
--- a/core/utils.py
+++ b/core/utils.py
@@ -45,6 +45,13 @@ sha = hashlib.new('sha256')
 COOKIE_MAX_AGE = 12*60*60 # seconds
 throw = 35.0
 
+DEV_OK = """On branch master
+Your branch is ahead of 'origin/master' by 1 commit.
+  (use "git push" to publish your local commits)
+
+nothing to commit, working tree clean
+"""
+
 class DatabaseResetOngoing(Exception):
     """Exception class for errors while the server is reimporting everything"""
 
@@ -126,7 +133,7 @@ def make_new_expo_dir(year):
 def current_expo():
     """Returns the current expo year, but also checks if the most recent expo year is the same 
     as this year. If it is not, then it creates an empty Expedition and fixes some files and 
-    folders. If were are more than one year out of date, it creates all intervening Expo objects
+    folders. If we are more than one year out of date, it creates all intervening Expo objects
     and folders. You will need to tidy this up manually.
     """
     expos = Expedition.objects.all().order_by('-year')
@@ -191,61 +198,10 @@ def parse_aliases(aliasfile):
         return [(None, None)], "Fail on file reading"
     return aliases, report
 
-def only_commit(fname, message, editor=None):
-    """Only used to commit a survex file edited and saved in view/survex.py"""
-    git = settings.GIT
-    cwd = fname.parent
-    filename = fname.name
-    # print(f'{fname=} ')
-    if editor:
-        editor =  git_string(editor)
-    else:
-        # cannot happen as form verification has this as an obligatory field
-        editor = "Anathema Device <a.device@potatohut.expo>"
-        
-    try:
-        print(f"git add {filename}")
-        cp_add = subprocess.run([git, "add", filename], cwd=cwd, capture_output=True, text=True)
-        if cp_add.returncode != 0:
-            msgdata = f"Ask a nerd to fix this problem in only_commit().\n--{cp_add.stderr}\n--{cp_add.stdout}\n--return code:{str(cp_add.returncode)}"
-            raise WriteAndCommitError(
-                f"CANNOT git ADD on server for this file {filename}. Edits saved but not added to git.\n\n" + msgdata
-            )
-        print(f"git commit {filename}")
-        print(f"Committing:\n{message=}\n{editor=}")
-        cmd_commit = [git, "commit", "-m", message, "--author", f"{editor}"]
- 
-        cp_commit = subprocess.run(cmd_commit, cwd=cwd, capture_output=True, text=True)
-        # This produces return code = 1 if it commits OK, but when the local repo still needs to be pushed to origin/loser
-        # which will be the case when running a test troggle system on a development machine
-        devok_text = """On branch master
-Your branch is ahead of 'origin/master' by 1 commit.
-  (use "git push" to publish your local commits)
-
-nothing to commit, working tree clean
-"""
-        if cp_commit.returncode == 1 and cp_commit.stdout == devok_text:
-            pass
-        else:
-            if cp_commit.returncode != 0 and not cp_commit.stdout.strip().endswith(
-                "nothing to commit, working tree clean"
-            ):
-                msgdata = f'--Ask a nerd to fix this problem in only_commit().\n--{cp_commit.stderr}\n--"{cp_commit.stdout}"\n--return code:{str(cp_commit.returncode)}'
-                print(msgdata)
-                raise WriteAndCommitError(
-                    f"Error code with git on server for this file {filename}. Edits saved, added to git, but NOT committed.\n\n"
-                    + msgdata
-                )
-
-    except subprocess.SubprocessError:
-        msg =  f"CANNOT git COMMIT on server for this file {filename}. Subprocess error. Edits not saved.\nAsk a nerd to fix this."
-        print(msg)
-        raise WriteAndCommitError(msg)
-           
 
 def git_string(author_string):
     """Rewrites the supplied editor string intoa git-complient author string
-    Uses a regular expression for a git-compatible author string
+    Uses a regular expression for a git-compatible author string written mostly by Copilot
     valid example "John Doe <john.doe@example.com>"
     """
     author_regex = re.compile(r'^[a-zA-Z][\w\s\_\.\-]* <[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-_]+\.[a-zA-Z]{2,}>$')
@@ -259,25 +215,103 @@ def git_string(author_string):
         editor += f" <{editor}@potatohut.expo>"
         print(f"Not git-compatible author string, replacing as '{editor}'")
         return editor
+        
+
+def git_add(filename, cwd, commands=[]):
+    """Add a file to the list of Staged files ready for a later git commit
+    """ 
+    git = settings.GIT
+    
+    # what is the purpose of this 'git diff' ? To prevent merge conflicts happening I guess, 
+    # so we do not have to reverse a 'git add'
+    print(f"git diff {filename}")
+    cmd_diff = [git, "diff", filename]
+    commands.append(cmd_diff)
+    cp_diff = subprocess.run(cmd_diff, cwd=cwd, capture_output=True, text=True)
+    if cp_diff.returncode != 0:
+        msgdata = f"Ask a nerd to fix this DIFF problem in git_add().\n--{cp_diff.stderr}\n--{cp_diff.stdout}\n--return code:{str(cp_diff.returncode)}"
+        raise WriteAndCommitError(
+            f"CANNOT git ADD on server for this file {filename}.\n\n" + msgdata
+        )
+    
+    print(f"git add {filename}")
+    cmd_add = [git, "add", filename]
+    commands.append(cmd_add)
+    cp_add = subprocess.run(cmd_add, cwd=cwd, capture_output=True, text=True)
+    if cp_add.returncode != 0:
+        msgdata = f"Ask a nerd to fix this ADD problem in git_add().\n--{cp_add.stderr}\n--{cp_add.stdout}\n--return code:{str(cp_add.returncode)}"
+        raise WriteAndCommitError(
+            f"CANNOT git ADD on server for this file {filename}.\n\n" + msgdata
+        )     
+    return commands
+
+
+def git_commit(cwd, message, editor, commands=[]):
+    """Commits whatever has been Staged by git in this directory
+    """
+    git = settings.GIT    
+    print(f"git commit in {cwd}")
+    print(f"Committing:\n{message=}\n{editor=}")
+    cmd_commit = [git, "commit", "-m", message, "--author", f"{editor}"]
+    commands.append(cmd_commit)
+
+    cp_commit = subprocess.run(cmd_commit, cwd=cwd, capture_output=True, text=True)
+    # This produces return code = 1 if it commits OK, but when the local repo still needs to be pushed to origin/repo
+    # which will be the case when running a test troggle system on a development machine
+    if cp_commit.returncode == 1 and cp_commit.stdout == DEV_OK: # only good for 1 commit ahead of origin/repo
+        pass
+    else:
+        if cp_commit.returncode != 0 and not cp_commit.stdout.strip().endswith(
+            "nothing to commit, working tree clean"
+        ):
+            msgdata = f'--Ask a nerd to fix this problem in add_commit().\n--{cp_commit.stderr}\n--"{cp_commit.stdout}"\n--return code:{str(cp_commit.returncode)}'
+            print(msgdata)
+            raise WriteAndCommitError(
+                f"Error code with git on server in this directory: {cwd}. Edits saved, added to git, but NOT committed.\n\n"
+                + msgdata
+            )
+    return commands
+    
+def add_commit(fname, message, editor=None):
+    """Only used to commit a survex file edited and saved in view/survex.py"""
+    cwd = fname.parent
+    filename = fname.name
+    commands = []
+ 
+    if editor:
+        editor =  git_string(editor)
+    else:
+        # 'cannot happen' as form verification has this as an obligatory field
+        editor = "Anathema Device <a.device@potatohut.expo>"
+        
+    try:
+        commands = git_add(filename, cwd, commands)
+        commands = git_commit(cwd, message, editor, commands)
+ 
+    except subprocess.SubprocessError:
+        msg =  f"CANNOT git ADD or COMMIT on server for this file {filename}.\nSubprocess error: {commands}\nEdits probably not saved.\nAsk a nerd to fix this."
+        print(msg)
+        raise WriteAndCommitError(msg)
+
 
 def write_and_commit(files, message, editor=None):
     """Writes the content to the filepath and adds and commits the file to git.  If this fails, a WriteAndCommitError is raised.
 
-    This  needs refactoring to just write and then call only_commit()
+    This  needs refactoring to just write and then call add_commit()
     """
+    # GIT see also core/views/uploads.py dwgupload()
+    # GIT see also core/views/expo.py editexpopage()
     git = settings.GIT
     commands = []
     if editor:
         editor =  git_string(editor)
     else:
         # cannot happen as form verification has this as an obligatory field
-        editor = "Automatic <automaton@potatohut.expo>"
+        editor = "Write_and_commit <automaton@potatohut.expo>"
     try:
         for filepath, content, encoding in files:
             cwd = filepath.parent
             filename = filepath.name
-            # GIT see also core/views/uploads.py dwgupload()
-            # GIT see also core/views/expo.py editexpopage()
             os.makedirs(os.path.dirname(filepath), exist_ok = True)
             if filepath.is_dir():
                 raise WriteAndCommitError(
@@ -301,7 +335,10 @@ def write_and_commit(files, message, editor=None):
             except Exception as e:
                 raise WriteAndCommitError(
                     f"CANNOT  write this file {filepath}. Ask a nerd to fix this: {e}"
-                )            
+                )      
+
+            # what is the purpose of this 'git diff' ? To prevent merge conflicts happening I guess, 
+            # so we do not have to reverse a 'git add'
             cmd_diff = [git, "diff", filename]
             cp_diff = subprocess.run(cmd_diff, cwd=cwd, capture_output=True, text=True)
             commands.append(cmd_diff)
@@ -328,41 +365,27 @@ def write_and_commit(files, message, editor=None):
                 print(f"No change {filepath}")
         filepaths = [filepath for filepath, content, encoding in files]
         # message = message + " " + str(filepaths)
-        print([git, "commit", "-m", message, "--author", f"{editor}"])
-        cmd_commit = [git, "commit", "-m", message, "--author", f"{editor}"]
-        cm_status = subprocess.run(cmd_commit, cwd=cwd, capture_output=True, text=True)
-        commands.append(cmd_commit)
-        if cm_status.returncode != 0:
-            msgdata = (
-                "Commands: " + str(commands) +
-                "Ask a nerd to fix this.\n\n"
-                + "Stderr: " + cm_status.stderr
-                + "\n\n"
-                + "Stdout: " + cm_status.stdout
-                + "\n\nreturn code: " + str(cm_status.returncode)
-                + "\n\ngit add return code in previous operation was: " + git_add_returncode
-            )
-            raise WriteAndCommitError(
-                f"ERROR committing. Edits saved, [maybe] added to git, but NOT committed.\n\n"
-                + msgdata
-            )     
-        cmd_status = [git, "status"] # + filepaths   
-        cp_status = subprocess.run(cmd_status, cwd=cwd, capture_output=True, text=True)
-        commands.append(cp_status)
-        #This produces return code = 1 if it commits OK, but when the repo still needs to be pushed to origin/expoweb
-        if (not cp_status.stdout) or len(cp_status.stdout) < 2 or cp_status.stdout.split("\n")[-2] != "nothing to commit, working tree clean":
-            msgdata = (
-                str(commands) +
-                "Ask a nerd to fix this.\n\n"
-                + "Stderr: " + cp_status.stderr
-                + "\n\n"
-                + "Stdout: " + cp_status.stdout
-                + "\n\nreturn code: " + str(cp_status.returncode)
-            )
-            raise WriteAndCommitError(
-                f"Error code with git on server for this file {filename}. Edits saved, added to git, but NOT committed. Git status not clean.\n\n"
-                + msgdata
-            )
+        
+        commands = git_commit(cwd, message, editor, commands)
+        
+        if False:
+            cmd_status = [git, "status"] # + filepaths   
+            cp_status = subprocess.run(cmd_status, cwd=cwd, capture_output=True, text=True)
+            commands.append(cp_status)
+            #This produces return code = 1 if it commits OK, but when the repo still needs to be pushed to origin/expoweb
+            if (not cp_status.stdout) or len(cp_status.stdout) < 2 or cp_status.stdout.split("\n")[-2] != "nothing to commit, working tree clean":
+                msgdata = (
+                    str(commands) +
+                    "Ask a nerd to fix this.\n\n"
+                    + "Stderr: " + cp_status.stderr
+                    + "\n\n"
+                    + "Stdout: " + cp_status.stdout
+                    + "\n\nreturn code: " + str(cp_status.returncode)
+                )
+                raise WriteAndCommitError(
+                    f"Error code with git on server for this file {filename}. Edits saved, added to git, but NOT committed. Git status not clean.\n\n"
+                    + msgdata
+                )
     except subprocess.SubprocessError:
         raise WriteAndCommitError(
             f"CANNOT git on server for this file {filename}. Subprocess error. Edits not saved.\nAsk a nerd to fix this."
@@ -387,13 +410,13 @@ def writetrogglefile(filepath, filecontent, commit_msg=None):
     core/models/caves.py
     and by
     make_new_expo_dir(year)
+    but NOT by core/views/caves.py
     
     Commit the new saved file to git
-    Callers to cave.writeDataFile() or entrance.writeDataFile() should handle the exception PermissionsError explicitly
+    
     """
     # GIT see also core/views/expo.py editexpopage()
     # GIT see also core/views/uploads.py dwgupload()
-    # Called from core/models/caves.py Cave.writeDataFile() Entrance.writeDataFile()
     filepath = Path(filepath)
     cwd = filepath.parent
     filename = filepath.name
diff --git a/core/views/caves.py b/core/views/caves.py
index 890782f..c91db76 100644
--- a/core/views/caves.py
+++ b/core/views/caves.py
@@ -20,7 +20,7 @@ from troggle.core.forms import CaveForm, EntranceForm, EntranceLetterForm  # Cav
 from troggle.core.models.caves import Cave, CaveAndEntrance, Entrance, GetCaveLookup, get_cave_leniently
 from troggle.core.models.logbooks import QM
 from troggle.core.models.wallets import Wallet
-from troggle.core.utils import current_expo, write_and_commit
+from troggle.core.utils import COOKIE_MAX_AGE, WriteAndCommitError, current_expo, git_string, write_and_commit
 from troggle.core.views import expo
 from troggle.parsers.caves import read_cave, read_entrance
 from troggle.settings import CAVEDESCRIPTIONS, ENTRANCEDESCRIPTIONS
@@ -400,12 +400,14 @@ def cavepage(request, karea=None, subpath=None):
     kpath = karea + subpath
     #print(f" ! cavepage:'{kpath}' kataster area:'{karea}' rest of path:'{subpath}'")
     
-    caves = Cave.objects.filter(url=kpath)
+    # replace this with .count()
+    caves = Cave.objects.filter(url=kpath) 
     if len(caves) == 1:
         cave = caves[0]
         return rendercave(request, cave, cave.slug())
 
 
+    # HORRIBLE HACK, to be removed..
     subpath = subpath.strip("//")
     # re do all this using pathlib functions
     parts = subpath.strip("/").split("/")
@@ -453,12 +455,19 @@ def cavepage(request, karea=None, subpath=None):
 def edit_cave(request, path="", slug=None):
     """This is the form that edits all the cave data and writes out an XML file in the :expoweb: repo folder
     The format for the file being saved is in templates/dataformat/cave.xml
-    Warning. This uses Django deep magic in the CaveForm processing.
 
     It saves the data into into the database and into the html file, which it then commits to git.
     
     We basically ignore the <path> as the <slug> is of the format 1624-114 and contains the area code
+    
+    Warning. This uses Django deep magic in the CaveForm processing.
+    See https://docs.djangoproject.com/en/5.1/topics/forms/modelforms/
+    https://django-formset.fly.dev/styling/
+    which generates the HTML form fields and also manages the syntax validation.
+    
+    See class CaveForm(ModelForm) in troggle/core/forms.py
     """
+    
     print(f"edit_cave(): {path=} {slug=}")
     message = ""
     if slug is None:
@@ -467,11 +476,17 @@ def edit_cave(request, path="", slug=None):
         print(f"{slug=}")
         if not (cave:= get_cave_from_slug(slug)): # walrus operator
             return render(request, "errors/badslug.html", {"badslug": f"for cave {caveslug} - from edit_cave()"})
+
+    print(f"Reading cookie...")
+    editor_id = request.COOKIES.get('editor_id', 'H&ouml;hlenforscher <hohlenforscher@stonebridge.expo>') # if no cookie, then default string
+    editor =  git_string(editor_id) # belt and braces, should have been validity checked on saving already
+    print(f"Cookie read: {editor_id=}  reformatted as: {editor=}")
         
     if request.POST:
         form = CaveForm(request.POST, instance=cave)
         if form.is_valid(): 
             print(f'edit_cave(): POST is valid. Editing {cave}')
+            editor = form.cleaned_data["who_are_you"]
             cave = form.save(commit=False)
             # print(cave)
             if not cave.filename:
@@ -486,6 +501,14 @@ def edit_cave(request, path="", slug=None):
                 cs = CaveSlug(cave=cave, slug=slug, primary=True)
                 print(f"edit_cave(): New CaveSlug saved {slug}")
                 cs.save()
+
+            if cave.entrances().count() > 0:
+                # Redirect after POST
+                edit_response = HttpResponseRedirect("/" + cave.url)
+            else:
+                edit_response = HttpResponseRedirect(reverse("newentrance", args = [cave.url_parent(), cave.slug()])) 
+            edit_response.set_cookie('editor_id', editor, max_age=COOKIE_MAX_AGE) # cookie expires after COOKIE_MAX_AGE seconds
+
             try:
                 cave_file = cave.file_output()
                 write_and_commit([cave_file], f"Online edit of cave {cave}")
@@ -493,16 +516,18 @@ def edit_cave(request, path="", slug=None):
             except PermissionError:
                 message = f"CANNOT  save this file.\nPERMISSIONS incorrectly set on server for this file {cave.filename}. Ask a nerd to fix this."
                 return render(request, "errors/generic.html", {"message": message})
-            except subprocess.SubprocessError:
-                message = f"CANNOT git on server for this file {cave.filename}. Edits may not be committed.\nAsk a nerd to fix this."
-                return render(request, "errors/generic.html", {"message": message})
+            except WriteAndCommitError as e:
+                message = f"CANNOT git on server for this file {cave.filename}.\n{e}\nEdits may not be committed.\nAsk a nerd to fix this."
+                return render(request, "errors/generic.html", {"message": e.message})
+            except subprocess.SubprocessError as e:
+                message = f"CANNOT update server for this file {cave.filename}.\n{e}\nEdits may not be committed.\nAsk a nerd to fix this."
+                return render(request, "errors/generic.html", {"message": message})               
             except:
                 raise
-            if cave.entrances().count() > 0:
-                return HttpResponseRedirect("/" + cave.url)
-            else:
-                return HttpResponseRedirect(reverse("newentrance", args = [cave.url_parent(), cave.slug()]))
+            print(f"Returning response now, which should set cookie on client browser")
+            return edit_response
 
+    # if a GET; and also falls-through from the POST handling to refresh the page
     else:
         if slug is not None:
             # re-read cave data from file.
@@ -515,9 +540,9 @@ def edit_cave(request, path="", slug=None):
                     print(f"edit_cave(): EXCEPTION attempting to read_cave({cave.filename})\n{e}")
                     raise
         
-            form = CaveForm(instance=cave, initial={'cave_slug': cave.slug()})
+            form = CaveForm(instance=cave, initial={'cave_slug': cave.slug(), "who_are_you":editor})
         else:
-            form = CaveForm()
+            form = CaveForm(initial={"who_are_you":editor})
             
     # The way formsets are rendered changed between Django 4 and Django 5
     major, _, _, _, _ = django.VERSION
@@ -622,6 +647,11 @@ def edit_entrance(request, path="", caveslug=None, entslug=None):
     print(f"{cave=}")
     imgpath = Path(path) / cave.areacode / cave.number()
     print(f"Edit Entrance {imgpath=}")
+
+    print(f"Reading cookie...")
+    editor_id = request.COOKIES.get('editor_id', 'Hohlenforscher <hohlenforscher@stonebridge.expo>') # if no cookie, then default string
+    editor =  git_string(editor_id) # belt and braces, should have been validity checked on saving already
+    print(f"Cookie read: {editor_id=}  reformatted as: {editor=}")
        
     if request.POST:
         print(f"POST Online edit of entrance: '{entrance}' where {cave=}")
@@ -656,6 +686,7 @@ def edit_entrance(request, path="", caveslug=None, entslug=None):
         else: 
             
             print(f"- POST {caveslug=} {entslug=} {entranceletter=} {path=}")
+            editor = entform.cleaned_data["who_are_you"]
             if entslug is None:
                 # we are creating a new entrance
                 entrance = entform.save(commit=False)
@@ -702,7 +733,7 @@ def edit_entrance(request, path="", caveslug=None, entslug=None):
             
             print(f"- POST WRITE letter: '{ce}' {entrance=}")
             try:
-                write_and_commit([entrance_file, cave_file], f"Online edit of entrance {entrance.slug}")
+                write_and_commit([entrance_file, cave_file], f"Online edit of entrance {entrance.slug}", editor)
                 return HttpResponseRedirect("/" + cave.url)
             except Exception as e:
                 efilepath, econtent, eencoding = entrance_file
diff --git a/core/views/survex.py b/core/views/survex.py
index 07eab38..c909a40 100644
--- a/core/views/survex.py
+++ b/core/views/survex.py
@@ -19,7 +19,7 @@ from troggle.core.models.caves import Cave, GetCaveLookup
 from troggle.core.models.logbooks import LogbookEntry
 from troggle.core.models.survex import SurvexBlock, SurvexFile  #, SurvexDirectory
 from troggle.core.models.wallets import Wallet
-from troggle.core.utils import COOKIE_MAX_AGE, current_expo, git_string, only_commit
+from troggle.core.utils import COOKIE_MAX_AGE, current_expo, git_string, add_commit
 from troggle.parsers.survex import parse_one_file
 
 """Everything that views survexfiles 
@@ -234,7 +234,7 @@ class SvxForm(forms.Form):
         else:
             comment = f"Online survex edit: {self.data['filename']}.svx on dev machine '{socket.gethostname()}' "
         print(f"Committing file which has been saved {editor=}")
-        only_commit(fname, comment, editor)
+        add_commit(fname, comment, editor)
 
         msg = f"SAVED and committed to git (if there were differences)\nEdited by:{editor}"
         # should only call this is something changed