diff --git a/appview/db/pulls.go b/appview/db/pulls.go index 77419f4..14129cf 100644 --- a/appview/db/pulls.go +++ b/appview/db/pulls.go @@ -101,10 +101,14 @@ type PullComment struct { } func (p *Pull) LatestPatch() string { - latestSubmission := p.Submissions[len(p.Submissions)-1] + latestSubmission := p.Submissions[p.LastRoundNumber()] return latestSubmission.Patch } +func (p *Pull) LastRoundNumber() int { + return len(p.Submissions) - 1 +} + func (s PullSubmission) AsNiceDiff(targetBranch string) types.NiceDiff { patch := s.Patch diff --git a/appview/pages/funcmap.go b/appview/pages/funcmap.go index 1a8d4a8..bedeabd 100644 --- a/appview/pages/funcmap.go +++ b/appview/pages/funcmap.go @@ -1,6 +1,7 @@ package pages import ( + "errors" "fmt" "html" "html/template" @@ -121,6 +122,20 @@ func funcMap() template.FuncMap { "list": func(args ...any) []any { return args }, + "dict": func(values ...any) (map[string]any, error) { + if len(values)%2 != 0 { + return nil, errors.New("invalid dict call") + } + dict := make(map[string]any, len(values)/2) + for i := 0; i < len(values); i += 2 { + key, ok := values[i].(string) + if !ok { + return nil, errors.New("dict keys must be strings") + } + dict[key] = values[i+1] + } + return dict, nil + }, "i": func(name string, classes ...string) template.HTML { data, err := icon(name, classes) if err != nil { diff --git a/appview/pages/pages.go b/appview/pages/pages.go index 8e2a580..32a5015 100644 --- a/appview/pages/pages.go +++ b/appview/pages/pages.go @@ -576,6 +576,40 @@ func (p *Pages) RepoPullPatchPage(w io.Writer, params RepoPullPatchParams) error return p.execute("repo/pulls/patch", w, params) } +type PullResubmitParams struct { + LoggedInUser *auth.User + RepoInfo RepoInfo + Pull *db.Pull + SubmissionId int +} + +func (p *Pages) PullResubmitFragment(w io.Writer, params PullResubmitParams) error { + return p.executePlain("fragments/pullResubmit", w, params) +} + +type PullActionsParams struct { + LoggedInUser *auth.User + RepoInfo RepoInfo + Pull *db.Pull + RoundNumber int + MergeCheck types.MergeCheckResponse +} + +func (p *Pages) PullActionsFragment(w io.Writer, params PullActionsParams) error { + return p.executePlain("fragments/pullActions", w, params) +} + +type PullNewCommentParams struct { + LoggedInUser *auth.User + RepoInfo RepoInfo + Pull *db.Pull + RoundNumber int +} + +func (p *Pages) PullNewCommentFragment(w io.Writer, params PullNewCommentParams) error { + return p.executePlain("fragments/pullNewComment", w, params) +} + func (p *Pages) Static() http.Handler { sub, err := fs.Sub(Files, "static") if err != nil { diff --git a/appview/pages/templates/fragments/editRepoDescription.html b/appview/pages/templates/fragments/editRepoDescription.html index 20e3465..d0c094b 100644 --- a/appview/pages/templates/fragments/editRepoDescription.html +++ b/appview/pages/templates/fragments/editRepoDescription.html @@ -2,10 +2,10 @@
{{ end }} diff --git a/appview/pages/templates/fragments/pullActions.html b/appview/pages/templates/fragments/pullActions.html new file mode 100644 index 0000000..3ee8d2a --- /dev/null +++ b/appview/pages/templates/fragments/pullActions.html @@ -0,0 +1,72 @@ +{{ define "fragments/pullActions" }} + {{ $lastIdx := sub (len .Pull.Submissions) 1 }} + {{ $roundNumber := .RoundNumber }} + + {{ $isPushAllowed := .RepoInfo.Roles.IsPushAllowed }} + {{ $isMerged := .Pull.State.IsMerged }} + {{ $isClosed := .Pull.State.IsClosed }} + {{ $isOpen := .Pull.State.IsOpen }} + {{ $isConflicted := and .MergeCheck (or .MergeCheck.Error .MergeCheck.IsConflicted) }} + {{ $isPullAuthor := and .LoggedInUser (eq .LoggedInUser.Did .Pull.OwnerDid) }} + {{ $isLastRound := eq $roundNumber $lastIdx }} +
+
+
+ + {{ if and $isPushAllowed $isOpen $isLastRound }} + {{ $disabled := "" }} + {{ if $isConflicted }} + {{ $disabled = "disabled" }} + {{ end }} + + {{ end }} + + {{ if and $isPullAuthor $isOpen $isLastRound }} + + {{ end }} + + {{ if and $isPullAuthor $isPushAllowed $isOpen $isLastRound }} + + {{ end }} + + {{ if and $isPullAuthor $isPushAllowed $isClosed $isLastRound }} + + {{ end }} +
+
+{{ end }} + + diff --git a/appview/pages/templates/fragments/pullNewComment.html b/appview/pages/templates/fragments/pullNewComment.html new file mode 100644 index 0000000..ba23f85 --- /dev/null +++ b/appview/pages/templates/fragments/pullNewComment.html @@ -0,0 +1,32 @@ +{{ define "fragments/pullNewComment" }} +
+
+ {{ didOrHandle .LoggedInUser.Did .LoggedInUser.Handle }} +
+
+ + + +
+
+
+{{ end }} + diff --git a/appview/pages/templates/fragments/pullResubmit.html b/appview/pages/templates/fragments/pullResubmit.html new file mode 100644 index 0000000..151cebc --- /dev/null +++ b/appview/pages/templates/fragments/pullResubmit.html @@ -0,0 +1,52 @@ +{{ define "fragments/pullResubmit" }} +
+ +
+ {{ i "pencil" "w-4 h-4" }} + resubmit your patch +
+ +
+ 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. +
+ +
+
+ + + +
+ +
+
+
+
+{{ end }} diff --git a/appview/pages/templates/fragments/repoDescription.html b/appview/pages/templates/fragments/repoDescription.html index 0ae3acd..9f292d0 100644 --- a/appview/pages/templates/fragments/repoDescription.html +++ b/appview/pages/templates/fragments/repoDescription.html @@ -8,8 +8,7 @@ {{ if .RepoInfo.Roles.IsOwner }} {{ end }} diff --git a/appview/pages/templates/repo/pulls/pull.html b/appview/pages/templates/repo/pulls/pull.html index 578085c..682184b 100644 --- a/appview/pages/templates/repo/pulls/pull.html +++ b/appview/pages/templates/repo/pulls/pull.html @@ -115,10 +115,12 @@ {{ end }} - {{ block "mergeStatus" $ }} {{ end }} + {{ if eq $lastIdx .RoundNumber }} + {{ block "mergeStatus" $ }} {{ end }} + {{ end }} {{ if $.LoggedInUser }} - {{ block "actions" (list $ .ID) }} {{ end }} + {{ template "fragments/pullActions" (dict "LoggedInUser" $.LoggedInUser "Pull" $.Pull "RepoInfo" $.RepoInfo "RoundNumber" .RoundNumber "MergeCheck" $.MergeCheck) }} {{ else }}
@@ -132,7 +134,16 @@ {{ end }} {{ define "mergeStatus" }} - {{ if .Pull.State.IsMerged }} + {{ if .Pull.State.IsClosed }} +
+
+
+ {{ i "ban" "w-4 h-4" }} + closed without merging +
+
+ {{ else if .Pull.State.IsMerged }}
@@ -141,6 +152,14 @@ >
+ {{ else if and .MergeCheck .MergeCheck.Error }} +
+
+
+ {{ i "triangle-alert" "w-4 h-4" }} + {{ .MergeCheck.Error }} +
+
{{ else if and .MergeCheck .MergeCheck.IsConflicted }}
@@ -170,70 +189,6 @@ {{ end }} {{ end }} -{{ define "actions" }} - {{ $rootObj := index . 0 }} - {{ $submissionId := index . 1 }} - - {{ with $rootObj }} - {{ $isPushAllowed := .RepoInfo.Roles.IsPushAllowed }} - {{ $isMerged := .Pull.State.IsMerged }} - {{ $isClosed := .Pull.State.IsClosed }} - {{ $isOpen := .Pull.State.IsOpen }} - {{ $isConflicted := and .MergeCheck .MergeCheck.IsConflicted }} - {{ $isPullAuthor := and .LoggedInUser (eq .LoggedInUser.Did .Pull.OwnerDid) }} -
-
-
- - {{ if and $isPushAllowed $isOpen }} - {{ $disabled := "" }} - {{ if $isConflicted }} - {{ $disabled = "disabled" }} - {{ end }} - - {{ end }} - - {{ if and $isPullAuthor $isOpen }} - - {{ end }} - - {{ if and $isPullAuthor $isPushAllowed $isOpen }} - - {{ end }} - - {{ if and $isPullAuthor $isPushAllowed $isClosed }} - - {{ end }} -
-
- {{ end }} -{{ end }} - {{ define "newComment" }} {{ $rootObj := index . 0 }} {{ $submissionId := index . 1 }} diff --git a/appview/state/pull.go b/appview/state/pull.go index 218801f..f0eb0ee 100644 --- a/appview/state/pull.go +++ b/appview/state/pull.go @@ -20,6 +20,48 @@ import ( lexutil "github.com/bluesky-social/indigo/lex/util" ) +// htmx fragment +func (s *State) PullActions(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case http.MethodGet: + user := s.auth.GetUser(r) + f, err := fullyResolvedRepo(r) + if err != nil { + log.Println("failed to get repo and knot", err) + return + } + + pull, ok := r.Context().Value("pull").(*db.Pull) + if !ok { + log.Println("failed to get pull") + s.pages.Notice(w, "pull-error", "Failed to edit patch. Try again later.") + return + } + + roundNumberStr := chi.URLParam(r, "round") + roundNumber, err := strconv.Atoi(roundNumberStr) + if err != nil { + roundNumber = pull.LastRoundNumber() + } + if roundNumber >= len(pull.Submissions) { + http.Error(w, "bad round id", http.StatusBadRequest) + log.Println("failed to parse round id", err) + return + } + + mergeCheckResponse := s.mergeCheck(f, pull) + + s.pages.PullActionsFragment(w, pages.PullActionsParams{ + LoggedInUser: user, + RepoInfo: f.RepoInfo(s, user), + Pull: pull, + RoundNumber: roundNumber, + MergeCheck: mergeCheckResponse, + }) + return + } +} + func (s *State) RepoSinglePull(w http.ResponseWriter, r *http.Request) { user := s.auth.GetUser(r) f, err := fullyResolvedRepo(r) @@ -62,37 +104,7 @@ func (s *State) RepoSinglePull(w http.ResponseWriter, r *http.Request) { } } - var mergeCheckResponse types.MergeCheckResponse - - // Only perform merge check if the pull request is not already merged - if pull.State != db.PullMerged { - secret, err := db.GetRegistrationKey(s.db, f.Knot) - if err != nil { - log.Printf("failed to get registration key for %s", f.Knot) - s.pages.Notice(w, "pull", "Failed to load pull request. Try again later.") - return - } - - ksClient, err := NewSignedClient(f.Knot, secret, s.config.Dev) - if err == nil { - resp, err := ksClient.MergeCheck([]byte(pull.LatestPatch()), pull.OwnerDid, f.RepoName, pull.TargetBranch) - if err != nil { - log.Println("failed to check for mergeability:", err) - } else { - respBody, err := io.ReadAll(resp.Body) - if err != nil { - log.Println("failed to read merge check response body") - } else { - err = json.Unmarshal(respBody, &mergeCheckResponse) - if err != nil { - log.Println("failed to unmarshal merge check response", err) - } - } - } - } else { - log.Printf("failed to setup signed client for %s; ignoring...", f.Knot) - } - } + mergeCheckResponse := s.mergeCheck(f, pull) s.pages.RepoSinglePull(w, pages.RepoSinglePullParams{ LoggedInUser: user, @@ -103,6 +115,63 @@ func (s *State) RepoSinglePull(w http.ResponseWriter, r *http.Request) { }) } +func (s *State) mergeCheck(f *FullyResolvedRepo, pull *db.Pull) types.MergeCheckResponse { + if pull.State == db.PullMerged { + return types.MergeCheckResponse{} + } + + secret, err := db.GetRegistrationKey(s.db, f.Knot) + if err != nil { + log.Printf("failed to get registration key: %w", err) + return types.MergeCheckResponse{ + Error: "failed to check merge status: this knot is unregistered", + } + } + + ksClient, err := NewSignedClient(f.Knot, secret, s.config.Dev) + if err != nil { + log.Printf("failed to setup signed client for %s; ignoring: %v", f.Knot, err) + return types.MergeCheckResponse{ + Error: "failed to check merge status", + } + } + + resp, err := ksClient.MergeCheck([]byte(pull.LatestPatch()), pull.OwnerDid, f.RepoName, pull.TargetBranch) + if err != nil { + log.Println("failed to check for mergeability:", err) + switch resp.StatusCode { + case 400: + return types.MergeCheckResponse{ + Error: "failed to check merge status: does this knot support PRs?", + } + default: + return types.MergeCheckResponse{ + Error: "failed to check merge status: this knot is unreachable", + } + } + } + + respBody, err := io.ReadAll(resp.Body) + if err != nil { + log.Println("failed to read merge check response body") + return types.MergeCheckResponse{ + Error: "failed to check merge status: knot is not speaking the right language", + } + } + defer resp.Body.Close() + + var mergeCheckResponse types.MergeCheckResponse + err = json.Unmarshal(respBody, &mergeCheckResponse) + if err != nil { + log.Println("failed to unmarshal merge check response", err) + return types.MergeCheckResponse{ + Error: "failed to check merge status: knot is not speaking the right language", + } + } + + return mergeCheckResponse +} + func (s *State) RepoPullPatch(w http.ResponseWriter, r *http.Request) { user := s.auth.GetUser(r) f, err := fullyResolvedRepo(r) @@ -213,7 +282,23 @@ func (s *State) PullComment(w http.ResponseWriter, r *http.Request) { return } + roundNumberStr := chi.URLParam(r, "round") + roundNumber, err := strconv.Atoi(roundNumberStr) + if err != nil || roundNumber >= len(pull.Submissions) { + http.Error(w, "bad round id", http.StatusBadRequest) + log.Println("failed to parse round id", err) + return + } + switch r.Method { + case http.MethodGet: + s.pages.PullNewCommentFragment(w, pages.PullNewCommentParams{ + LoggedInUser: user, + RepoInfo: f.RepoInfo(s, user), + Pull: pull, + RoundNumber: roundNumber, + }) + return case http.MethodPost: body := r.FormValue("body") if body == "" { @@ -221,13 +306,6 @@ func (s *State) PullComment(w http.ResponseWriter, r *http.Request) { return } - submissionIdstr := r.FormValue("submissionId") - submissionId, err := strconv.Atoi(submissionIdstr) - if err != nil { - s.pages.Notice(w, "pull", "Invalid comment submission.") - return - } - // Start a transaction tx, err := s.db.BeginTx(r.Context(), nil) if err != nil { @@ -263,6 +341,7 @@ func (s *State) PullComment(w http.ResponseWriter, r *http.Request) { }, }, }) + log.Println(atResp.Uri) if err != nil { log.Println("failed to create pull comment", err) s.pages.Notice(w, "pull-comment", "Failed to create comment.") @@ -276,7 +355,7 @@ func (s *State) PullComment(w http.ResponseWriter, r *http.Request) { PullId: pull.PullId, Body: body, CommentAt: atResp.Uri, - SubmissionId: submissionId, + SubmissionId: pull.Submissions[roundNumber].ID, }) if err != nil { log.Println("failed to create pull comment", err) @@ -433,6 +512,12 @@ func (s *State) ResubmitPull(w http.ResponseWriter, r *http.Request) { } switch r.Method { + case http.MethodGet: + s.pages.PullResubmitFragment(w, pages.PullResubmitParams{ + RepoInfo: f.RepoInfo(s, user), + Pull: pull, + }) + return case http.MethodPost: patch := r.FormValue("patch") diff --git a/appview/state/router.go b/appview/state/router.go index d919e59..33f586a 100644 --- a/appview/state/router.go +++ b/appview/state/router.go @@ -66,13 +66,27 @@ func (s *State) UserRouter() http.Handler { r.Route("/{pull}", func(r chi.Router) { r.Use(ResolvePull(s)) r.Get("/", s.RepoSinglePull) - r.Get("/round/{round}", s.RepoPullPatch) + + r.Route("/round/{round}", func(r chi.Router) { + r.Get("/", s.RepoPullPatch) + r.Get("/actions", s.PullActions) + r.Route("/comment", func(r chi.Router) { + r.Get("/", s.PullComment) + r.Post("/", s.PullComment) + }) + }) // authorized requests below this point r.Group(func(r chi.Router) { r.Use(AuthMiddleware(s)) - r.Post("/resubmit", s.ResubmitPull) - r.Post("/comment", s.PullComment) + r.Route("/resubmit", func(r chi.Router) { + r.Get("/", s.ResubmitPull) + r.Post("/", s.ResubmitPull) + }) + r.Route("/comment", func(r chi.Router) { + r.Get("/", s.PullComment) + r.Post("/", s.PullComment) + }) r.Post("/close", s.ClosePull) r.Post("/reopen", s.ReopenPull) // collaborators only