Show commit build status in pull request Activities #3028
Service Desk opened 3 weeks ago

Follow-up from issue #3027.

Motivation

On the pull request Activities tab, each update lists the commits from that push. To see build status for a commit, you currently have to open the project Commits page (or the commit itself). The PR header already shows jobs for the current head, but not for the other commits in the activity history.

Expected behavior

  • Show the same overall commit build-status control used on the project Commits page next to each commit listed in PR Activities (including older updates and rebased commits).
  • Place the icon beside the commit message (same place as on the Commits page).
  • Clicking it should open the job list for that commit (same as Commits), in the context of this pull request.
  • If the commit has no jobs, hide the control (same as Commits).

Scope notes

  • Status meaning matches the Commits page: overall status of all jobs for that commit.
  • Interaction matches the Commits page: overall status icon; click opens the job list (and can run jobs).
  • Show the control on every commit listed in Activities, including older pushes and rebased commits (rebased rows are already marked as rebased).

Implementation plan

This is a UI reuse of the existing commit-status widget. No new model, REST API, or build-query logic is required. Status data for Activities commits is already prefetched.

Current behavior (do not redo)

  • Each push is rendered by PullRequestUpdatePanel as a list of commits: author, message, signature, date, hash, copy. There is no build-status control.
  • PullRequestActivitiesPage already collects every update commit id and calls project.cacheCommitStatuses(buildService.queryStatus(project, commitIds)) on first render and when new update activities arrive. CommitStatusLink reads that cache via Project.getCommitStatuses.
  • The Commits page (CommitListPanel) adds CommitStatusLink next to each commit message. That widget shows an overall status icon, hides itself when the commit has no jobs, and opens JobListPanel (inspect / run jobs).
  • The PR header PullRequestJobsPanel is out of scope: it only shows jobs for the current head / merge-check, not per-commit history.

Files to change

  1. server-core/src/main/java/io/onedev/server/web/page/project/pullrequests/detail/activities/activity/PullRequestUpdatePanel.html

    • In each commit row, add a buildStatus link immediately after the message component and before signature (agreed placement: beside the message, not by the hash, and not between signature and date).
    • Suggested markup: <a wicket:id="buildStatus" class="mr-2 flex-shrink-0"></a> so spacing matches the other row chips and a long message does not squeeze the icon.
  2. server-core/src/main/java/io/onedev/server/web/page/project/pullrequests/detail/activities/activity/PullRequestUpdatePanel.java

    • In populateItem, add CommitStatusLink for that row’s commit, using commit.copy() as the commit id (same as CommitListPanel).
    • Wire it as follows:
      • Project: getUpdate().getRequest().getTargetProject() — same project already used for commit links and for the Activities status cache.
      • Pull request: the current request. CommitStatusLink uses one getPullRequest() for both overall status and the job-list dropdown. Passing the request makes “Run job” create PR-associated builds and makes the dropdown query builds in this PR (the “in the context of this pull request” requirement).
      • refName: null — same as the project Commits page, so status is not limited to one branch ref.
      • getSeenBranchTip: null — same as other PR job-run controls on PullRequestDetailPage.
    • CommitStatusLink.onConfigure already hides the control when the commit’s build spec has no jobs. Do not add a second visibility rule.
    • Apply this to every displayed commit, including rebased rows (pendingCommits already marks those rows; do not skip the widget).
  3. server-core/src/main/java/io/onedev/server/web/page/project/pullrequests/detail/activities/pull-request-activities.css — only if layout/QA shows a problem.

    • Rebased-row rules set muted color on nested a/span. Status icon colors use !important (build-status-successful, etc.), so they should stay readable. If the dropdown arrow or spacing looks wrong on dense/rebased rows, add a narrow exception (for example keep .commit-status from shrinking or from inheriting muted color on the arrow). Do not change rebased styling for message/hash.

Do not change PullRequestActivitiesPage status prefetch unless a new code path lists commits that are not already in commitIds. Do not change CommitStatusLink, JobListPanel, or the Commits page. Do not add status to PullRequestChangePanel (merge-commit hash in a change event is not the Activities commit list). No translation keys: the widget already owns its tooltip strings.

Status / job-list semantics (decision)

Project.getCommitStatuses(commitId, request, refName) filters stored build infos by request id and optional ref. The Commits page passes request = null, so it shows builds not tied to a pull request.

This feature is specified both as “same overall control as Commits” and “job list in the context of this pull request”. Those share one getPullRequest() on CommitStatusLink. Use the current pull request so running a job from Activities stays on the PR. Overall status is then the overall status of jobs stored for that commit and this request. Do not split the widget or add a second status API.

If a commit has jobs in the build spec but no PR-associated builds yet, the icon still shows (same as Commits when jobs exist and status is empty). If the build spec has no jobs, the control is hidden.

Live updates

CommitStatusLink / BuildStatusIcon already observe Build.getCommitStatusChangeObservable(projectId, commitHash). No extra ChangeObserver is needed on the update panel for status. The existing PR ChangeObserver on the panel continues to cover rebase/pending-commit row state.

Display limits

WebConstants.MAX_DISPLAY_COMMITS (500) already truncates a single update’s list. Only rendered rows need a widget. Prefetch may still cache statuses for commits not shown; that is existing behavior and can stay.

Out of scope

  • Per-job icons on each commit row (rejected in issue #3027).
  • Status only on head commit or only on non-rebased commits (rejected).
  • Changing PR header required-job chips.
  • server-ee.

Test plan

  1. Open a PR whose updates have CI. On Activities, each listed commit shows the overall status icon beside the message.
  2. Click the icon: the same job list dropdown as on Commits opens; jobs can be inspected and run; a run is associated with this PR.
  3. A commit whose build spec has no jobs shows no control.
  4. After a force-push, rebased rows stay marked rebased and still show status for the old hashes.
  5. An update with more than 500 commits still shows the existing “too many commits” notice; the displayed tail has status icons.
  6. While a job is running, the icon updates without a full page reload.
  7. Cross-project PR: icons resolve against the target project (where update commits are read and statuses are cached).
  8. Narrow / lg breakpoint: the icon stays next to the message and does not jump into the hash/actions cluster (d-none d-lg-block).
  9. Dark mode: status colors remain correct on normal and rebased rows.
  • Zak Siddiqui commented 3 days ago

    @servicedesk it would be nice if you could do an ASCII text render of the current and proposed.

    @robin please have a look at this when you get a chance.

  • Service Desk commented 3 days ago

    @zaakiy ASCII render of current vs proposed on the pull request Activities tab. The status control is the same widget as the project Commits page: overall icon plus dropdown arrow; click opens the job list.

    Current (no status)

    robin added 2 commits                                         2 hours ago
    |
    |  [o] (av) Fix login timeout on idle sessions  (sig)  2 hours ago     a1b2c3d ⧉
    |  [o] (av) Update session idle handling        (sig)  2 hours ago     d4e5f6a ⧉
    

    Each row today is: commit icon, author avatar, message, signature, date, then hash and copy on the right. There is no build-status control.

    Proposed (status beside the message)

    robin added 2 commits                                         2 hours ago
    |
    |  [o] (av) Fix login timeout on idle sessions  [ok▼]  (sig)  2 hours ago     a1b2c3d ⧉
    |  [o] (av) Update session idle handling        [ok▼]  (sig)  2 hours ago     d4e5f6a ⧉
    

    The overall status icon sits immediately after the message and before the signature. Click [ok▼] opens the same job-list dropdown as on Commits, in this pull request:

                                          ┌──────────────────────────┐
                                          │ CI                       │
                                          │   Unit Test        [ok]  │
                                          │   Integration      [ok]  │
                                          │   Publish          [run] │
                                          └──────────────────────────┘
    

    Other cases

    |  [o] (av) Add docs for the new API            [fail▼] (sig)  1 day ago      b7c8d9e ⧉
    |  [o] (av) Tweak README (no jobs in spec)              (sig)  1 day ago      f0a1b2c ⧉
    |  [o] (av) Old commit after force-push         [ok▼]   (sig)  3 days ago     1122334 ⧉
    
    • Failed commit: red/failed overall icon, same placement.
    • No jobs in the build spec: control is hidden (same as Commits).
    • Rebased row after a force-push: still marked rebased (muted), and still shows status for the old hash.

    This matches the plan already on the issue: reuse CommitStatusLink on every listed commit, hide it when there are no jobs, and pass this pull request so Run job stays on the PR.

    Please say if you want the icon moved or any of these cases drawn differently.

  • Zak Siddiqui commented 3 days ago

    @servicedesk the OK status should actually be tick-circle-o, to be consistent with how it's done elsewhere

  • Service Desk commented 3 days ago

    @zaakiy Updated the ASCII to use the same status icons as elsewhere. Successful is tick-circle-o (plus the dropdown arrow), not an OK label. Failed is times-circle-o. That is already what the existing commit-status widget renders, so the implementation plan is unchanged.

    Proposed (status beside the message)

    robin added 2 commits                                         2 hours ago
    |
    |  [o] (av) Fix login timeout on idle sessions  tick-circle-o▼  (sig)  2 hours ago     a1b2c3d ⧉
    |  [o] (av) Update session idle handling        tick-circle-o▼  (sig)  2 hours ago     d4e5f6a ⧉
    

    Click tick-circle-o▼ opens the same job-list dropdown as on Commits:

                                          ┌─────────────────────────────────┐
                                          │ CI                              │
                                          │   Unit Test        tick-circle-o │
                                          │   Integration      tick-circle-o │
                                          │   Publish          [run]         │
                                          └─────────────────────────────────┘
    

    Other cases

    |  [o] (av) Add docs for the new API            times-circle-o▼ (sig)  1 day ago      b7c8d9e ⧉
    |  [o] (av) Tweak README (no jobs in spec)               (sig)  1 day ago      f0a1b2c ⧉
    |  [o] (av) Old commit after force-push         tick-circle-o▼  (sig)  3 days ago     1122334 ⧉
    
    • Successful: tick-circle-o plus dropdown arrow, same placement after the message.
    • Failed: times-circle-o plus dropdown arrow.
    • No jobs in the build spec: control is hidden (same as Commits).
    • Rebased row after a force-push: still marked rebased (muted), and still shows status for the old hash.
1/1
Type
New Feature
Priority
Normal
Assignee
Labels
No labels
Issue Votes (0)
Watchers (3)
Reference
OD-3028
Please wait...
Connection lost or session expired, reload to recover
Page is in error, reload to recover