diff --git a/appview/pages/templates/repo/pulls/single.html b/appview/pages/templates/repo/pulls/single.html index 99606e7a..6af38a4e 100644 --- a/appview/pages/templates/repo/pulls/single.html +++ b/appview/pages/templates/repo/pulls/single.html @@ -135,7 +135,7 @@ {{ $seeAll = not .ChangeId }} {{ else }} {{ $version := index .Pull.Versions .VersionId }} - {{ $seeAll = and (eq .DiffParams.Base $version.Base) (eq .DiffParams.Head $version.Head) }} + {{ $seeAll = and (eq .DiffParams.Base "base") (eq .DiffParams.Head "head") }} {{ end }}
@@ -155,7 +155,7 @@
{{ range .Commits }} {{ $messageParts := splitN .Message "\n\n" 2 }} - {{ $active := and (not $seeAll) (eq $.ActiveCommitId .Hash.String) }} + {{ $active := and (not $seeAll) (eq (shortId $.ActiveCommitId) (shortId .Hash.String)) }} {{ $toggleId := printf "commit-message-toggle-%s" .Hash }} {{ $pipeline := index $.Pipelines .Hash.String }}
@@ -167,9 +167,9 @@
{{ end }}
- {{ slice .Hash.String 0 8 }} + {{ shortId .Hash.String }} {{ if .ChangeId }} - {{ slice .ChangeId 0 8 }} + {{ shortId .ChangeId }} {{ end }} {{ if $.IsInterdiff }} {{ index $messageParts 0 }} @@ -200,7 +200,6 @@
- {{ $diffUrl := "" }} {{ if not .IsInterdiff }} {{ $diffUrl = (printf "/%s/pulls/%d/diff?view=unified" .Pull.RepoDid .Pull.PullId) }} diff --git a/appview/pulls/diff.go b/appview/pulls/diff.go index ea3a92e9..b1b6fef2 100644 --- a/appview/pulls/diff.go +++ b/appview/pulls/diff.go @@ -8,10 +8,12 @@ import ( "io" "log/slog" "net/http" + "strconv" "strings" "github.com/bluesky-social/indigo/atproto/syntax" "golang.org/x/sync/errgroup" + "tangled.org/core/appview/models" "tangled.org/core/appview/pages" gitmirrorv1 "tangled.org/core/gitmirror/proto/gen" ) @@ -21,26 +23,39 @@ func (s *Pulls) PullDiffFragment(w http.ResponseWriter, r *http.Request) { l := s.logger.With("handler", "PullDiffFragment") ctx := r.Context() + pull, ok := r.Context().Value("pull").(*models.Pull) + if !ok { + l.Error("failed to get pull") + http.Error(w, "failed to get PR", http.StatusInternalServerError) + return + } + var ( - repoRaw = r.URL.Query().Get("repo") + version = r.URL.Query().Get("version") base = r.URL.Query().Get("base") // base commit ID head = r.URL.Query().Get("head") // head commit ID unified = r.URL.Query().Get("view") == "unified" ) - repo, err := syntax.ParseDID(repoRaw) + versionId, err := strconv.Atoi(version) if err != nil { - http.Error(w, "invalid repo DID", http.StatusBadRequest) + http.Error(w, "invalid version id", http.StatusBadRequest) return } - l.Debug("diff fragment", "base", base, "head", head) + l.Debug("diff fragment", "pull", pull.AtUri(), "base", base, "head", head) var params pages.PullDiffFragmentParams - params.Repo = repo + params.Repo = pull.RepoDid params.DiffBase = base params.DiffHead = head params.DiffUrl = r.URL.Path params.Unified = unified - params.Files, params.ErrorMsg = s.diffFragmentParams(ctx, l, repo, base, head, unified) + if base == "base" { + base = pull.TargetBranch + } + if head == "head" { + head = pull.Versions[versionId].Head + } + params.Files, params.ErrorMsg = s.diffFragmentParams(ctx, l, pull.RepoDid, base, head, unified) if err := s.pages.PullDiffFragment(w, params); err != nil { l.Error("failed to render", "err", err) } diff --git a/appview/pulls/single.go b/appview/pulls/single.go index df1f7330..76a92864 100644 --- a/appview/pulls/single.go +++ b/appview/pulls/single.go @@ -1,6 +1,7 @@ package pulls import ( + "cmp" "context" "errors" "fmt" @@ -63,16 +64,10 @@ func (s *Pulls) PullDiff(w http.ResponseWriter, r *http.Request) { l = l.With("user", user.Did) } - f, err := s.repoResolver.Resolve(r) - if err != nil { - l.Error("failed to get repo and knot", "err", err) - return - } - pull, ok := r.Context().Value("pull").(*models.Pull) if !ok { l.Error("failed to get pull") - s.pages.Error500(w) + http.Error(w, "failed to get PR", http.StatusInternalServerError) return } @@ -113,7 +108,7 @@ func (s *Pulls) PullDiff(w http.ResponseWriter, r *http.Request) { // defer render var params pages.PullDiffParams - params.PullPageBaseParams = s.makePullPageBaseParams(r, user, f, pull) + params.PullPageBaseParams = s.makePullPageBaseParams(r, user, pull) params.VersionId = version.ID defer func() { if err := s.pages.PullDiff(w, params); err != nil { @@ -121,29 +116,8 @@ func (s *Pulls) PullDiff(w http.ResponseWriter, r *http.Request) { } }() - // special cases - // default to {target}..{current.head} - if diffBase == "" || diffBase == "base" { - diffBase = version.Base - } - if diffHead == "" || diffHead == "head" { - diffHead = version.Head - } - params.DiffParams.Base = diffBase - params.DiffParams.Head = diffHead - - // TODO: Ideally we should show diff between .., - // - but we can't find target branch's commit from forked repo. - // - and we will get 0 diff when PR is merged. - // so for now, we show diff between .. - // // resolve target branch -> (branch, commit) - // xrpcc := &indigoxrpc.Client{Host: s.config.KnotMirror.Url} - // branch, err := tangled.GitTempGetBranch(ctx, xrpcc, pull.TargetBranch, pull.RepoDid.String()) - // if err != nil { - // l.Warn("Failed to resolve target branch", "branch", pull.TargetBranch, "err", err) - // params.ErrorMsg = fmt.Sprintf("Failed to resolve target branch %q", pull.TargetBranch) - // return - // } + params.DiffParams.Base = cmp.Or(diffBase, "base") + params.DiffParams.Head = cmp.Or(diffHead, "head") commits, err := s.listCommits(ctx, pull.SourceRepo, version.Base, version.Head) if err != nil { l.Error("failed to list commits", "err", err) @@ -157,7 +131,7 @@ func (s *Pulls) PullDiff(w http.ResponseWriter, r *http.Request) { for i, commit := range params.Commits { shas[i] = commit.Hash.String() } - params.Pipelines = fetchPipelines(ctx, l, f, shas) + params.Pipelines = fetchPipelines(ctx, l, pull.Repo, shas) } // PullInterDiff is router for /pulls/{pull}/{version}..{version}/{change} @@ -174,12 +148,6 @@ func (s *Pulls) PullInterDiff(w http.ResponseWriter, r *http.Request) { l = l.With("user", user.Did) } - f, err := s.repoResolver.Resolve(r) - if err != nil { - l.Error("failed to get repo and knot", "err", err) - return - } - pull, ok := r.Context().Value("pull").(*models.Pull) if !ok { s.logger.Error("failed to get pull") @@ -212,7 +180,7 @@ func (s *Pulls) PullInterDiff(w http.ResponseWriter, r *http.Request) { // defer render var params pages.PullInterdiffParams - params.PullPageBaseParams = s.makePullPageBaseParams(r, user, f, pull) + params.PullPageBaseParams = s.makePullPageBaseParams(r, user, pull) params.Version1 = version1 params.Version2 = version2 params.ChangeId = changeId @@ -222,18 +190,6 @@ func (s *Pulls) PullInterDiff(w http.ResponseWriter, r *http.Request) { } }() - // TODO: Ideally we should show diff between .., - // - but we can't find target branch's commit from forked repo. - // - and we will get 0 diff when PR is merged. - // so for now, we show diff between .. - // // resolve target branch -> (branch, commit) - // xrpcc := &indigoxrpc.Client{Host: s.config.KnotMirror.Url} - // branch, err := tangled.GitTempGetBranch(ctx, xrpcc, pull.TargetBranch, pull.RepoDid.String()) - // if err != nil { - // l.Warn("Failed to resolve target branch", "branch", pull.TargetBranch, "err", err) - // params.ErrorMsg = fmt.Sprintf("Failed to resolve target branch %q", pull.TargetBranch) - // return - // } var commits1, commits2 []types.Commit g, gctx := errgroup.WithContext(ctx) if changeId != "" { @@ -258,7 +214,7 @@ func (s *Pulls) PullInterDiff(w http.ResponseWriter, r *http.Request) { for i, commit := range params.Commits { shas[i] = commit.Hash.String() } - params.Pipelines = fetchPipelines(ctx, l, f, shas) + params.Pipelines = fetchPipelines(ctx, l, pull.Repo, shas) if changeId != "" { // interdiff by change-id @@ -361,7 +317,7 @@ func (s *Pulls) PullPatchRaw(w http.ResponseWriter, r *http.Request) { w.Write(rawOut) } -func (s *Pulls) makePullPageBaseParams(r *http.Request, user *oauth.MultiAccountUser, f *models.Repo, pull *models.Pull) pages.PullPageBaseParams { +func (s *Pulls) makePullPageBaseParams(r *http.Request, user *oauth.MultiAccountUser, pull *models.Pull) pages.PullPageBaseParams { l := s.logger ctx := r.Context() @@ -386,7 +342,7 @@ func (s *Pulls) makePullPageBaseParams(r *http.Request, user *oauth.MultiAccount labelDefs, err := db.GetLabelDefinitions( s.db, - orm.FilterIn("at_uri", f.Labels), + orm.FilterIn("at_uri", pull.Repo.Labels), orm.FilterContains("scope", tangled.RepoPullNSID), ) if err != nil {