From 6379061a0239a066ea2fe13034cdb18bf65e172e Mon Sep 17 00:00:00 2001 From: Anirudh Oppiliappan Date: Sat, 5 Apr 2025 16:29:09 +0300 Subject: [PATCH] appview: pulls: make target branch picking generic --- appview/pages/pages.go | 1 - .../fragments/pullCompareBranches.html | 49 ++++++---------- .../templates/fragments/pullCompareForks.html | 57 +++++++++++-------- .../fragments/pullCompareForksBranches.html | 16 ------ appview/pages/templates/repo/pulls/new.html | 35 ++++++++++-- appview/pages/templates/repo/pulls/pull.html | 7 ++- appview/state/pull.go | 30 +++++----- 7 files changed, 99 insertions(+), 96 deletions(-) diff --git a/appview/pages/pages.go b/appview/pages/pages.go index 843eb54d..a29e5c9a 100644 --- a/appview/pages/pages.go +++ b/appview/pages/pages.go @@ -575,7 +575,6 @@ func (p *Pages) SingleIssueCommentFragment(w io.Writer, params SingleIssueCommen type RepoNewPullParams struct { LoggedInUser *auth.User RepoInfo RepoInfo - Forks []db.Repo Branches []types.Branch Active string } diff --git a/appview/pages/templates/fragments/pullCompareBranches.html b/appview/pages/templates/fragments/pullCompareBranches.html index b7e6dd44..d1b4fce4 100644 --- a/appview/pages/templates/fragments/pullCompareBranches.html +++ b/appview/pages/templates/fragments/pullCompareBranches.html @@ -1,35 +1,20 @@ {{ define "fragments/pullCompareBranches" }} - -
- -
- - - {{ i "move-left" "w-5 h-5" }} - - -
- -
+
+ +
+ {{ end }} diff --git a/appview/pages/templates/fragments/pullCompareForks.html b/appview/pages/templates/fragments/pullCompareForks.html index 71d82547..706bdb4e 100644 --- a/appview/pages/templates/fragments/pullCompareForks.html +++ b/appview/pages/templates/fragments/pullCompareForks.html @@ -3,32 +3,39 @@ -
- - - -
+
+
+ + + +
-
-
- Select a fork first to view available branches +
+
+ Select a fork first to view available branches +
diff --git a/appview/pages/templates/fragments/pullCompareForksBranches.html b/appview/pages/templates/fragments/pullCompareForksBranches.html index 0d2cdfe6..67227622 100644 --- a/appview/pages/templates/fragments/pullCompareForksBranches.html +++ b/appview/pages/templates/fragments/pullCompareForksBranches.html @@ -1,21 +1,5 @@ {{ define "fragments/pullCompareForksBranches" }}
- - - {{ i "move-left" "w-5 h-5" }} - - + + {{ range .Branches }} + + {{ end }} + +
+ +

Then, choose a pull strategy.

+ -
+
{{ template "fragments/pullPatchUpload" . }}
-
+
-
+
diff --git a/appview/pages/templates/repo/pulls/pull.html b/appview/pages/templates/repo/pulls/pull.html index 8878ab0c..7de9a8e8 100644 --- a/appview/pages/templates/repo/pulls/pull.html +++ b/appview/pages/templates/repo/pulls/pull.html @@ -50,8 +50,13 @@ {{ if not .Pull.IsSameRepoBranch }} {{ $owner }}/{{ .PullSourceRepo.Name }} {{ end }} + + {{ $fullRepo := .RepoInfo.FullName }} + {{ if not .Pull.IsSameRepoBranch }} + {{ $fullRepo = printf "%s/%s" $owner .PullSourceRepo.Name }} + {{ end }} - {{ .Pull.PullSource.Branch }} + {{ .Pull.PullSource.Branch }} {{ end }} diff --git a/appview/state/pull.go b/appview/state/pull.go index 73bcad32..82c99702 100644 --- a/appview/state/pull.go +++ b/appview/state/pull.go @@ -501,12 +501,6 @@ func (s *State) NewPull(w http.ResponseWriter, r *http.Request) { return } - forks, err := db.GetForksByDid(s.db, user.Did) - if err != nil { - log.Println("failed to get forks", err) - return - } - switch r.Method { case http.MethodGet: us, err := NewUnsignedClient(f.Knot, s.config.Dev) @@ -538,7 +532,6 @@ func (s *State) NewPull(w http.ResponseWriter, r *http.Request) { s.pages.RepoNewPull(w, pages.RepoNewPullParams{ LoggedInUser: user, RepoInfo: f.RepoInfo(s, user), - Forks: forks, Branches: result.Branches, }) case http.MethodPost: @@ -549,32 +542,37 @@ func (s *State) NewPull(w http.ResponseWriter, r *http.Request) { sourceBranch := r.FormValue("sourceBranch") patch := r.FormValue("patch") + // Validate required fields for all PR types + if title == "" || body == "" || targetBranch == "" { + s.pages.Notice(w, "pull", "Title, body and target branch are required.") + return + } + + // Determine PR type based on input parameters isPushAllowed := f.RepoInfo(s, user).Roles.IsPushAllowed() isBranchBased := isPushAllowed && sourceBranch != "" && fromFork == "" - isPatchBased := patch != "" isForkBased := fromFork != "" && sourceBranch != "" + isPatchBased := patch != "" && !isBranchBased && !isForkBased + // Validate we have at least one valid PR creation method if !isBranchBased && !isPatchBased && !isForkBased { s.pages.Notice(w, "pull", "Neither source branch nor patch supplied.") return } - if isBranchBased && isPatchBased { + // Can't mix branch-based and patch-based approaches + if isBranchBased && patch != "" { s.pages.Notice(w, "pull", "Cannot select both patch and source branch.") return } - if title == "" || body == "" || targetBranch == "" { - s.pages.Notice(w, "pull", "Title, body and target branch are required.") - return - } - + // Handle the PR creation based on the type if isBranchBased { s.handleBranchBasedPull(w, r, f, user, title, body, targetBranch, sourceBranch) - } else if isPatchBased { - s.handlePatchBasedPull(w, r, f, user, title, body, targetBranch, patch) } else if isForkBased { s.handleForkBasedPull(w, r, f, user, fromFork, title, body, targetBranch, sourceBranch) + } else if isPatchBased { + s.handlePatchBasedPull(w, r, f, user, title, body, targetBranch, patch) } return } -- 2.54.0