diff --git a/appview/pages/pages.go b/appview/pages/pages.go index 618ba51f..a7b18beb 100644 --- a/appview/pages/pages.go +++ b/appview/pages/pages.go @@ -1488,12 +1488,6 @@ type RepoSinglePullParams struct { ActiveRound int IsInterdiff bool - // WorkflowsChanged and ChangedWorkflowFiles describe whether the latest - // round's patch touches .tangled/workflows/, for warning maintainers - // before they manually trigger CI on a fork-based pull request. - WorkflowsChanged bool - ChangedWorkflowFiles []string - Reactions map[syntax.ATURI]map[models.ReactionKind]models.ReactionDisplayData UserReacted map[syntax.ATURI]map[models.ReactionKind]bool @@ -1528,6 +1522,13 @@ type PullActionsParams struct { BranchDeleteStatus *models.BranchDeleteStatus Stack models.Stack + // Workflow warning state for fork-based pulls without a pipeline on the + // latest commit. WorkflowsChanged and ChangedWorkflowFiles are computed + // from the latest round's patch. + WorkflowsChanged bool + ChangedWorkflowFiles []string + HasPipeline bool + // renders buttons in a pre-check state and attaches the hx-trigger="load" // that fetches the real, checked fragment Loading bool diff --git a/appview/pages/templates/repo/pulls/fragments/pullActions.html b/appview/pages/templates/repo/pulls/fragments/pullActions.html index eeb17627..c3afc2db 100644 --- a/appview/pages/templates/repo/pulls/fragments/pullActions.html +++ b/appview/pages/templates/repo/pulls/fragments/pullActions.html @@ -23,112 +23,219 @@ {{ $isLastRound := eq $roundNumber $lastIdx }} {{ $isSameRepoBranch := .Pull.IsBranchBased }} {{ $isUpToDate := .ResubmitCheck.No }} -
- {{ if .LoggedInUser }} - - {{ end }} - {{ if and (not $loading) .BranchDeleteStatus }} - + {{ if and (not $loading) $isLastRound }} + {{ if and $isOpen $isForkBased (not .HasPipeline) (ne .RepoInfo.Spindle "") }} + {{ template "workflowWarning" . }} + {{ end }} + {{ template "mergeCheck" . }} + {{ template "resubmitStatus" . }} {{ end }} - {{ if and $isPushAllowed $isOpen $isLastRound }} +
+ {{ if .LoggedInUser }} - {{ end }} - - {{ if and $isPullAuthor $isOpen $isLastRound }} - + {{ end }} + {{ if $showRunCI }} + {{ template "runCiModal" . }} + {{ end }} + {{ if and $isPushAllowed $isOpen $isLastRound }} + - {{ end }} + Merge{{if $stackCount}} {{$stackCount}}{{end}} + + {{ end }} - {{ if and (or $isPullAuthor $isPushAllowed) $isOpen $isLastRound }} - - {{ end }} + {{ if and $isPullAuthor $isOpen $isLastRound }} + - {{ end }} + hx-disabled-elt="#resubmitBtn" + class="btn-flat p-2 flex items-center gap-2 disabled:opacity-50 disabled:cursor-not-allowed group" + + {{ if $loading }} + title="Checking for updates…" + disabled + {{ else if $isUpToDate }} + title="Update this branch to resubmit this pull request" + disabled + {{ else }} + title="Resubmit this pull request" + {{ end }} + > + {{ if $loading }} + {{ i "loader-circle" "w-4 h-4 animate-spin" }} + {{ else }} + {{ i "rotate-ccw" "w-4 h-4 inline group-[.htmx-request]:hidden" }} + {{ i "loader-circle" "w-4 h-4 animate-spin hidden group-[.htmx-request]:inline" }} + {{ end }} + Resubmit + + {{ end }} + + {{ if and (or $isPullAuthor $isPushAllowed) $isOpen $isLastRound }} + + {{ end }} + + {{ if and (or $isPullAuthor $isPushAllowed) $isClosed $isLastRound }} + + {{ end }} +
+{{ end }} - {{ if and (not $loading) $isLastRound }} -
{{ template "mergeCheck" . }}
-
{{ template "resubmitStatus" . }}
+{{ define "workflowWarning" }} + {{ if .WorkflowsChanged }} +
+ + {{ i "triangle-alert" "w-4 h-4 text-amber-600 dark:text-amber-500" }} + Workflow files changed in this round +
+ Expand + +
+ + +
+ {{ else }} +
+ {{ i "triangle-alert" "w-4 h-4" }} + Workflow needs approval +
{{ end }} {{ end }} +{{ define "runCiModal" }} + + +
+
+ + {{ if .WorkflowsChanged }} +
+ {{ i "triangle-alert" "size-4" }} + Workflow files changed in this round +
+ +

Workflow files changed in this round. Are you sure you want to run CI?

+ {{ else }} +
+ {{ i "triangle-alert" "size-4" }} + Workflow needs approval +
+

This fork-based pull request needs approval to run CI. Are you sure?

+ {{ end }} +
+ + +
+
+
+
+{{ end }} + {{ define "mergeCheck" }} {{ $isOpen := .Pull.State.IsOpen }} {{ if and $isOpen .MergeCheck .MergeCheck.Error }} diff --git a/appview/pages/templates/repo/pulls/fragments/triggerCi.html b/appview/pages/templates/repo/pulls/fragments/triggerCi.html deleted file mode 100644 index 4d3c7cfd..00000000 --- a/appview/pages/templates/repo/pulls/fragments/triggerCi.html +++ /dev/null @@ -1,49 +0,0 @@ -{{ define "repo/pulls/fragments/runCiButton" }} - -{{ end }} - -{{ define "repo/pulls/fragments/triggerCi" }} -{{ if .WorkflowsChanged }} -
- -
- {{ i "triangle-alert" "w-4 h-4 flex-shrink-0" }} - Workflow files changed in this round -
- Expand - -
-
- - -
- {{ $confirm := printf "Workflow files changed in this round (%s). Review before running. Run anyway?" (join .ChangedWorkflowFiles ", ") }} - {{ template "repo/pulls/fragments/runCiButton" (dict "RepoInfo" .RepoInfo "Pull" .Pull "Confirm" $confirm) }} -
-
-{{ else }} -
- {{ i "circle-play" "w-4 h-4 flex-shrink-0" }} - CI hasn't run on the latest commit - {{ template "repo/pulls/fragments/runCiButton" (dict "RepoInfo" .RepoInfo "Pull" .Pull "Confirm" "") }} -
-{{ end }} -
-{{ end }} diff --git a/appview/pages/templates/repo/pulls/pull.html b/appview/pages/templates/repo/pulls/pull.html index ac5fac58..b5eefefc 100644 --- a/appview/pages/templates/repo/pulls/pull.html +++ b/appview/pages/templates/repo/pulls/pull.html @@ -342,16 +342,6 @@ {{ template "submissionInfo" $ }} {{ template "submissionCommits" $ }} {{ template "submissionPipeline" $ }} - {{ if eq $lastIdx $round }} -
- {{ if $root.Pull.State.IsOpen }} -
- {{ i "loader-circle" "w-4 h-4 animate-spin" }} - Checking mergeability… -
- {{ end }} -
- {{ end }} {{ end }} @@ -549,8 +539,6 @@ {{ end }} - {{ else if and $root.Pull.IsForkBased (eq $item.RoundNumber $root.Pull.LastRoundNumber) $root.RepoInfo.Roles.IsOwner }} - {{ template "repo/pulls/fragments/triggerCi" $root }} {{ end }} {{ end }} @@ -594,7 +582,6 @@
{{ if eq $lastIdx $item.RoundNumber }} {{ block "mergeStatus" $root }} {{ end }} -
{{ end }}
diff --git a/appview/pulls/single.go b/appview/pulls/single.go index c92ddb76..6dfcda5b 100644 --- a/appview/pulls/single.go +++ b/appview/pulls/single.go @@ -63,6 +63,27 @@ func (s *Pulls) PullActions(w http.ResponseWriter, r *http.Request) { // only the last round's buttons and banners use merge/resubmit checks isLastRound := roundNumber == pull.LastRoundNumber() branchDeleteStatus := s.branchDeleteStatus(r, f, pull) + + var workflowsChanged bool + var changedWorkflows []string + hasPipeline := false + if isLastRound && f.Spindle != "" { + pipelines, err := s.fetchPipelines(r.Context(), f.Spindle, f.RepoDid, []string{pull.LatestSha()}) + if err != nil { + l.Error("failed to fetch latest pipeline", "err", err) + } else if pipelines != nil { + _, hasPipeline = pipelines[pull.LatestSha()] + } + + if pull.IsForkBased() && !hasPipeline { + changedWorkflows, err = changedWorkflowFiles(pull.LatestSubmission().CombinedPatch()) + if err != nil { + l.Error("failed to inspect latest round's patch for workflow changes", "err", err) + } + workflowsChanged = len(changedWorkflows) > 0 + } + } + mergeCheckResponse := types.MergeCheckResponse{} resubmitResult := pages.Unknown if isLastRound { @@ -73,14 +94,17 @@ func (s *Pulls) PullActions(w http.ResponseWriter, r *http.Request) { } s.pages.PullActionsFragment(w, pages.PullActionsParams{ - BaseParams: pages.BaseParamsFromContext(r.Context()), - RepoInfo: s.repoResolver.GetRepoInfo(r, user), - Pull: pull, - RoundNumber: roundNumber, - MergeCheck: mergeCheckResponse, - ResubmitCheck: resubmitResult, - BranchDeleteStatus: branchDeleteStatus, - Stack: stack, + BaseParams: pages.BaseParamsFromContext(r.Context()), + RepoInfo: s.repoResolver.GetRepoInfo(r, user), + Pull: pull, + RoundNumber: roundNumber, + MergeCheck: mergeCheckResponse, + ResubmitCheck: resubmitResult, + BranchDeleteStatus: branchDeleteStatus, + Stack: stack, + WorkflowsChanged: workflowsChanged, + ChangedWorkflowFiles: changedWorkflows, + HasPipeline: hasPipeline, }) return } @@ -160,35 +184,12 @@ func (s *Pulls) repoPullHelper(w http.ResponseWriter, r *http.Request, interdiff shas = append(shas, p.LatestSha()) } - // commitId -> latest pipeline - pipelines := func(ctx context.Context) map[string]types.Pipeline { - m := make(map[string]types.Pipeline) - if f.Spindle == "" { - return m - } - spindleUrl, err := hostutil.EnsureHttpScheme(f.Spindle) - if err != nil { - l.Error("invalid spindle host", "host", f.Spindle, "err", err) - return m - } - xrpcc := &indigoxrpc.Client{Host: spindleUrl} - out, err := tangled.CiQueryPipelines(ctx, xrpcc, shas, "", nil, 0, f.RepoDid) - if err != nil { - l.Error("failed to fetch pipelines", "err", err) - return m - } - - return types.PipelinesByCommit(out.Pipelines) - }(r.Context()) - - var workflowsChanged bool - var changedWorkflows []string - if _, hasPipeline := pipelines[pull.LatestSha()]; pull.IsForkBased() && !hasPipeline { - changedWorkflows, err = changedWorkflowFiles(pull.LatestSubmission().CombinedPatch()) - if err != nil { - l.Error("failed to inspect latest round's patch for workflow changes", "err", err) - } - workflowsChanged = len(changedWorkflows) > 0 + pipelines, err := s.fetchPipelines(r.Context(), f.Spindle, f.RepoDid, shas) + if err != nil { + l.Error("failed to fetch pipelines", "err", err) + } + if pipelines == nil { + pipelines = make(map[string]types.Pipeline) } entities := []syntax.ATURI{pull.AtUri()} @@ -278,9 +279,6 @@ func (s *Pulls) repoPullHelper(w http.ResponseWriter, r *http.Request, interdiff ActiveRound: roundIdInt, IsInterdiff: interdiff, - WorkflowsChanged: workflowsChanged, - ChangedWorkflowFiles: changedWorkflows, - Reactions: reactions, UserReacted: userReactions, @@ -305,6 +303,22 @@ func (s *Pulls) combinedDiff(pull *models.Pull, round int) types.DiffRenderer { return diff } +func (s *Pulls) fetchPipelines(ctx context.Context, spindle string, repoDid string, shas []string) (map[string]types.Pipeline, error) { + if spindle == "" { + return nil, nil + } + spindleUrl, err := hostutil.EnsureHttpScheme(spindle) + if err != nil { + return nil, err + } + xrpcc := &indigoxrpc.Client{Host: spindleUrl} + out, err := tangled.CiQueryPipelines(ctx, xrpcc, shas, "", nil, 0, repoDid) + if err != nil { + return nil, err + } + return types.PipelinesByCommit(out.Pipelines), nil +} + func (s *Pulls) RepoSinglePull(w http.ResponseWriter, r *http.Request) { l := s.logger.With("handler", "RepoSinglePull") diff --git a/types/pipeline.go b/types/pipeline.go index 6dc25124..fa27cca6 100644 --- a/types/pipeline.go +++ b/types/pipeline.go @@ -103,8 +103,6 @@ func (w WorkflowStatus) Created() time.Time { return t } - - type Trigger struct { *tangled.CiPipeline_Trigger }