Show commit build status in pull request Activities (OD-3028)

Show commit build status in pull request Activities #3028

Service Desk opened 1 month ago

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

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.
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).
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

Open a PR whose updates have CI. On Activities, each listed commit shows the overall status icon beside the message.
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.
A commit whose build spec has no jobs shows no control.
After a force-push, rebased rows stay marked rebased and still show status for the old hashes.
An update with more than 500 commits still shows the existing “too many commits” notice; the displayed tail has status icons.
While a job is running, the icon updates without a full page reload.
Cross-project PR: icons resolve against the target project (where update commits are read and statuses are cached).
Narrow / lg breakpoint: the icon stays next to the message and does not jump into the hash/actions cluster (d-none d-lg-block).
Dark mode: status colors remain correct on normal and rebased rows.

Copy

Related
- Question about adding build status to activity history in pull requests (OD-3027) Closed

Activities

Service Desk referenced from other issue 1 month ago

Question about adding build status to activity history in pull requests (OD-3027)

Closed
Zak Siddiqui commented 2 weeks 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.

Copy

Service Desk commented 2 weeks 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.

Copy

Zak Siddiqui commented 2 weeks ago

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

Copy

Service Desk commented 2 weeks 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.

Copy

Zak Siddiqui commented 2 weeks ago
@servicedesk consider the plan in this issue against the latest codebase. My concern is the codebase may have changed, and this issue therefore may require changing or may, in fact, have been deprecated. Please advise on the course of action.

Don't assume. Ask high-level clarifying questions if needed.

Numbered questions (1, 2, 3, etc)

Lettered answer options (A, B, C, etc)

Include a markdown table that contains the following for each option:

number (with an emoji that represents your recommended option)

Option short title

Explanation.

Pros (with relevant emojis)

Cons (with relevant emojis)

Below the table, justify your recommendation (prefer that you elaborate instead of assuming knowledge).

Copy

Service Desk commented 2 weeks ago

@zaakiy I checked the plan against the current main codebase at 58b26ebac9f2e4cf4e2b650e85697d917e40786e (2026-09-07). The feature has not been implemented or deprecated, and the current issue plan remains actionable.

Current-code findings:

PullRequestUpdatePanel still renders author, message, signature, date, hash, and copy only; there is no per-commit build-status component.
PullRequestActivitiesPage still prefetches statuses for all update commit IDs on initial render and for newly arrived update activities. No new query path is needed.
CommitStatusLink is still the shared control used by the Commits page. It still computes overall status, opens JobListPanel, observes commit-status changes, and hides itself when the commit build spec has no jobs.
BuildStatusIcon still maps successful to tick-circle-o and failed to times-circle-o, so reusing the widget satisfies the icon consistency concern automatically.
A relevant change landed on 2026-09-03 for issue OD-3077: CommitStatusLink now requires getSeenBranchTip(String branch) to protect job runs from stale branch views. The issue plan already includes the required override returning null, matching the existing pull-request job-run control. No further adaptation is necessary.
The target-project, current-pull-request, null-ref, rebased-row, 500-commit limit, and live-update assumptions in the plan all still match the current implementations.

No high-level clarification is needed because the issue already explicitly resolves the only material product choice: status and the job dropdown share the current pull-request context, so runs remain PR-associated.

Number	Option short title	Explanation	Pros	Cons
1 ✅	Proceed with the current plan	Add `CommitStatusLink` beside each activity commit message, passing the target project, current pull request, a null ref, and a null seen-branch tip. Start without CSS changes and add a narrow rule only if visual QA identifies a problem.	✅ Small reuse-only change; ✅ matches current APIs; ✅ correct icons and live updates come from the existing widget; ✅ preserves PR-associated job runs	⚠️ Still requires UI QA for dense and rebased rows
2	Redesign status and run contexts	Split the widget so the icon can show non-PR commit status while the dropdown runs jobs in PR context.	🔧 Could mimic the Commits page status scope literally	❌ Conflicts with the recorded semantics decision; ❌ requires new widget/API behavior; ❌ materially increases scope and regression risk
3	Deprecate or close the issue	Take no implementation action on the assumption that newer code made the feature redundant.	🧹 No maintenance work	❌ The Activities rows still lack the requested control; ❌ does not meet the stated user need; ❌ unsupported by current code

I recommend Option 1. The current code preserves every reuse point the plan depends on, while the one recent interface change relevant to job launching is already reflected in the plan. This keeps the work narrowly scoped to the activity row markup and component wiring, avoids duplicating status/build logic, and retains the explicitly chosen PR context for inspecting and running jobs.

Copy

Zak Siddiqui commented 2 weeks ago

Please proceed with option one.

Copy
Service Desk commented 2 weeks ago

No comment
Service Desk commented 2 weeks ago

I’m relevant to the requested implementation, but I can’t proceed because my account does not have permission to write code in onedev/server. tod issue checkout --for-write 3028 returned HTTP 403: “No permission to write code in issue project.” Please grant the required code-write permission or reassign the implementation to someone who has it.

Copy
Zak Siddiqui commented 2 weeks ago

@robin

Copy
Robin Shen commented 1 week ago

@zaakiy commit status was displayed here in old versions, but removed as this is confusing with PR build statuses. Users ask why PR still needs build verification if all commits are displayed as verified there.

Copy
Service Desk commented 1 week ago

@robin You are right. I checked the history and found commit 33a971f086 (“Do not display build status on some pages to avoid confusion”), which explicitly removed CommitStatusLink from the pull-request update/activity rows. My earlier assessment checked whether the current code could support the proposed reuse, but missed that this exact UI had been intentionally removed.

Restoring it would reintroduce the ambiguity between per-commit build status and the pull requests own build-verification status. I therefore withdraw the recommendation to proceed with option 1 and recommend that the current implementation plan not be pursued. If the underlying need remains, it should first be reframed around a presentation that clearly distinguishes commit builds from PR verification. No checkout or code change was made.

Copy
Zak Siddiqui commented 1 week ago

If the underlying need remains, it should first be reframed around a presentation that clearly distinguishes commit builds from PR verification.

Very keen for this to be considered @robin

Copy
Robin Shen commented 1 week ago

It is not easy to clearly distinguishes that without extra explanation. It is just a click away to see build status of the commit.

Copy
Robin Shen changed state to 'Closed' 1 week ago
Previous Value Current Value
Open

Closed
Login to comment

Previous Value	Current Value
Open	Closed

Type	New Feature
Priority	Normal
Assignee	Robin Shen
Labels	No labels

Issue Votes (0)

Watchers (3)

Reference

OD-3028