From 26f6bc6b3bdc546a6c18d115017d74849ef26787 Mon Sep 17 00:00:00 2001
From: Anirudh Oppiliappan <x@icyphox.sh>
Date: Sun, 16 Mar 2025 19:38:37 +0200
Subject: [PATCH] appview/pulls: improve ui + validate patches

---
 .../pages/templates/repo/issues/issue.html    |  5 +-
 appview/pages/templates/repo/pulls/new.html   | 24 ++++---
 appview/pages/templates/repo/pulls/pull.html  | 67 ++++++++-----------
 appview/state/pull.go                         | 38 ++++++++++-
 appview/state/signer.go                       | 21 ++++--
 5 files changed, 98 insertions(+), 57 deletions(-)

diff --git a/appview/pages/templates/repo/issues/issue.html b/appview/pages/templates/repo/issues/issue.html
index 6cbca117..a32d2fbc 100644
--- a/appview/pages/templates/repo/issues/issue.html
+++ b/appview/pages/templates/repo/issues/issue.html
@@ -1,7 +1,4 @@
-{{ define "title" }}
-    {{ .Issue.Title }} &middot; issue #{{ .Issue.IssueId }} &middot;
-    {{ .RepoInfo.FullName }}
-{{ end }}
+{{ define "title" }}{{ .Issue.Title }} &middot; issue #{{ .Issue.IssueId }} &middot;{{ .RepoInfo.FullName }}{{ end }}
 
 {{ define "repoContent" }}
     <header class="pb-4">
diff --git a/appview/pages/templates/repo/pulls/new.html b/appview/pages/templates/repo/pulls/new.html
index b7f3ed13..4d09d809 100644
--- a/appview/pages/templates/repo/pulls/new.html
+++ b/appview/pages/templates/repo/pulls/new.html
@@ -15,15 +15,16 @@
                 <p class="text-gray-500">
                     The branch you want to make your change against.
                 </p>
-                <select name="targetBranch" class="p-1 border border-gray-200 bg-white">
-                  <option disabled selected>select a branch</option>
-                  {{ range .Branches }}
-                  <option
-                      value="{{ .Reference.Name }}"
-                      class="py-1">
-                      {{ .Reference.Name }}
-                  </option>
-                  {{ end }}
+                <select
+                    name="targetBranch"
+                    class="p-1 border border-gray-200 bg-white"
+                >
+                    <option disabled selected>select a branch</option>
+                    {{ range .Branches }}
+                        <option value="{{ .Reference.Name }}" class="py-1">
+                            {{ .Reference.Name }}
+                        </option>
+                    {{ end }}
                 </select>
             </div>
             <div>
@@ -44,7 +45,12 @@
                         rows="10"
                         class="w-full resize-y font-mono"
                         placeholder="Paste your git-format-patch output here."
+                        hx-post="/pulls/validate"
+                        hx-trigger="input changed delay:500ms"
+                        hx-target="#patch-validation"
+                        hx-include="[name='patch']"
                     ></textarea>
+                    <div id="pull-validate"></div>
                 </div>
             </div>
             <div>
diff --git a/appview/pages/templates/repo/pulls/pull.html b/appview/pages/templates/repo/pulls/pull.html
index 65c8b85f..466f2aa0 100644
--- a/appview/pages/templates/repo/pulls/pull.html
+++ b/appview/pages/templates/repo/pulls/pull.html
@@ -69,7 +69,7 @@
     {{ $isPullAuthor := and .LoggedInUser (eq .LoggedInUser.Did .Pull.OwnerDid) }}
     {{ $isPushAllowed := .RepoInfo.Roles.IsPushAllowed }}
 
-    {{ if $isPullAuthor }}
+    {{ if and $isPullAuthor (not .Pull.State.IsMerged) }}
     <section id="update-card" class="mt-8 space-y-4 relative">
       {{ block "resubmitCard" . }} {{ end }}
     </section>
@@ -87,24 +87,6 @@
       {{ end }}
     </section>
 
-    {{ if and (or $isPullAuthor $isPushAllowed) (not .Pull.State.IsMerged) }}
-        {{ $action := "close" }}
-        {{ $icon := "circle-x" }}
-        {{ $hoverColor := "red" }}
-        {{ if .Pull.State.IsClosed }}
-            {{ $action = "reopen" }}
-            {{ $icon = "circle-dot" }}
-            {{ $hoverColor = "green" }}
-        {{ end }}
-        <button
-          hx-post="/{{ .RepoInfo.FullName }}/pulls/{{ .Pull.PullId }}/{{ $action }}"
-          hx-swap="none"
-          class="btn mt-8 text-sm flex items-center gap-2">
-            <i data-lucide="{{ $icon }}" class="w-4 h-4 mr-2 text-{{ $hoverColor }}-400"></i>
-            <span class="text-black">{{ $action }}</span>
-        </button>
-    {{ end }}
-
     <div id="pull-close"></div>
     <div id="pull-reopen"></div>
 {{ end }}
@@ -202,21 +184,11 @@
   <div
       id="merge-status-card"
       class="rounded relative bg-purple-50 border border-purple-200 p-4">
-      {{ if gt (len .Comments) 0 }}
-          <div
-              class="absolute left-8 -top-4 w-px h-4 bg-gray-300"
-          ></div>
-      {{ else }}
-          <div
-              class="absolute left-8 -top-8 w-px h-8 bg-gray-300"
-          ></div>
-      {{ end }}
-
 
       <div class="flex items-center gap-2 text-purple-500">
           <i data-lucide="git-merge" class="w-4 h-4"></i>
           <span class="font-medium"
-              >Pull request successfully merged</span
+              >pull request successfully merged</span
           >
       </div>
 
@@ -263,6 +235,7 @@
 
 {{ define "noConflictsCard" }}
   {{ $isPushAllowed := .RepoInfo.Roles.IsPushAllowed }}
+  {{ $isPullAuthor := and .LoggedInUser (eq .LoggedInUser.Did .Pull.OwnerDid) }}
   <div
       id="merge-status-card"
       class="rounded relative border bg-green-50 border-green-200 p-4">
@@ -291,6 +264,24 @@
           </button>
         {{ end }}
 
+        {{ if and (or $isPullAuthor $isPushAllowed) (not .Pull.State.IsMerged) }}
+          {{ $action := "close" }}
+          {{ $icon := "circle-x" }}
+          {{ $hoverColor := "red" }}
+          {{ if .Pull.State.IsClosed }}
+              {{ $action = "reopen" }}
+              {{ $icon = "circle-dot" }}
+              {{ $hoverColor = "green" }}
+          {{ end }}
+          <button
+            hx-post="/{{ .RepoInfo.FullName }}/pulls/{{ .Pull.PullId }}/{{ $action }}"
+            hx-swap="none"
+            class="btn mt-4 flex items-center gap-2">
+              <i data-lucide="{{ $icon }}" class="w-4 h-4 text-{{ $hoverColor }}-400"></i>
+              <span class="text-black">{{ $action }}</span>
+          </button>
+        {{ end }}
+
         <div id="pull-merge-error" class="error"></div>
         <div id="pull-merge-success" class="success"></div>
       </div>
@@ -304,21 +295,21 @@
 
       <div class="flex items-center gap-2 text-amber-500">
         <i data-lucide="edit" class="w-4 h-4"></i>
-        <span class="font-medium">Resubmit your patch</span>
+        <span class="font-medium">resubmit your patch</span>
       </div>
 
       <div class="mt-2 text-sm text-gray-700">
-        You can update this patch to address reviews if any. 
-        This begins a new round of reviews,
-        you can still view your previous submissions and reviews.
+        You can update this patch to address any reviews. 
+        This will begin a new round of reviews,
+        but you'll still be able to view your previous submissions and feedback.
       </div>
 
-      <div class="mt-4 flex items-center gap-2">
-        <form hx-post="/{{ .RepoInfo.FullName }}/pulls/{{ .Pull.PullId }}/resubmit" class="w-full">
+      <div class="mt-4 flex flex-col">
+        <form hx-post="/{{ .RepoInfo.FullName }}/pulls/{{ .Pull.PullId }}/resubmit" class="w-full" hx-swap="none">
           <textarea
             name="patch"
-            class="w-full p-2 rounded border border-gray-200"
-            placeholder="Enter new patch"
+            class="w-full p-2 mb-2 rounded border border-gray-200"
+            placeholder="Paste your updated patch here."
             ></textarea>
           <button
               type="submit"
diff --git a/appview/state/pull.go b/appview/state/pull.go
index c91ad1a8..25042a3b 100644
--- a/appview/state/pull.go
+++ b/appview/state/pull.go
@@ -7,6 +7,7 @@ import (
 	"log"
 	"net/http"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/sotangled/tangled/api/tangled"
@@ -300,6 +301,12 @@ func (s *State) NewPull(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 
+		// Validate patch format
+		if !isPatchValid(patch) {
+			s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.")
+			return
+		}
+
 		tx, err := s.db.BeginTx(r.Context(), nil)
 		if err != nil {
 			log.Println("failed to start tx")
@@ -392,6 +399,12 @@ func (s *State) ResubmitPull(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 
+		// Validate patch format
+		if !isPatchValid(patch) {
+			s.pages.Notice(w, "resubmit-error", "Invalid patch format. Please provide a valid diff.")
+			return
+		}
+
 		tx, err := s.db.BeginTx(r.Context(), nil)
 		if err != nil {
 			log.Println("failed to start tx")
@@ -478,7 +491,7 @@ func (s *State) MergePull(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// Merge the pull request
-	resp, err := ksClient.Merge([]byte(pull.LatestPatch()), user.Did, f.RepoName, pull.TargetBranch)
+	resp, err := ksClient.Merge([]byte(pull.LatestPatch()), user.Did, f.RepoName, pull.TargetBranch, pull.Title, pull.Body, "", "")
 	if err != nil {
 		log.Printf("failed to merge pull request: %s", err)
 		s.pages.Notice(w, "pull-merge-error", "Failed to merge pull request. Try again later.")
@@ -607,3 +620,26 @@ func (s *State) ReopenPull(w http.ResponseWriter, r *http.Request) {
 	s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d", f.OwnerSlashRepo(), pull.PullId))
 	return
 }
+
+// Very basic validation to check if it looks like a diff/patch
+// A valid patch usually starts with diff or --- lines
+func isPatchValid(patch string) bool {
+	// Basic validation to check if it looks like a diff/patch
+	// A valid patch usually starts with diff or --- lines
+	if len(patch) == 0 {
+		return false
+	}
+
+	lines := strings.Split(patch, "\n")
+	if len(lines) < 2 {
+		return false
+	}
+
+	// Check for common patch format markers
+	firstLine := strings.TrimSpace(lines[0])
+	return strings.HasPrefix(firstLine, "diff ") ||
+		strings.HasPrefix(firstLine, "--- ") ||
+		strings.HasPrefix(firstLine, "Index: ") ||
+		strings.HasPrefix(firstLine, "+++ ") ||
+		strings.HasPrefix(firstLine, "@@ ")
+}
diff --git a/appview/state/signer.go b/appview/state/signer.go
index 626ccfa8..311c5314 100644
--- a/appview/state/signer.go
+++ b/appview/state/signer.go
@@ -10,6 +10,8 @@ import (
 	"net/http"
 	"net/url"
 	"time"
+
+	"github.com/sotangled/tangled/types"
 )
 
 type SignerTransport struct {
@@ -156,16 +158,25 @@ func (s *SignedClient) AddCollaborator(ownerDid, repoName, memberDid string) (*h
 	return s.client.Do(req)
 }
 
-func (s *SignedClient) Merge(patch []byte, ownerDid, targetRepo, branch string) (*http.Response, error) {
+func (s *SignedClient) Merge(
+	patch []byte,
+	ownerDid, targetRepo, branch, commitMessage, commitBody, authorName, authorEmail string,
+) (*http.Response, error) {
 	const (
 		Method = "POST"
 	)
 	endpoint := fmt.Sprintf("/%s/%s/merge", ownerDid, targetRepo)
+	
+	mr := types.MergeRequest{
+		Branch: branch,
+		CommitMessage: commitMessage,
+		CommitBody: commitBody,
+		AuthorName: authorName,
+		AuthorEmail: authorEmail,
+		Patch: string(patch),
+	}
 
-	body, _ := json.Marshal(map[string]interface{}{
-		"patch":  string(patch),
-		"branch": branch,
-	})
+	body, _ := json.Marshal(mr)
 
 	req, err := s.newRequest(Method, endpoint, body)
 	if err != nil {
-- 
2.51.2