Skip to content

Commit

Permalink
Fix hidden commit status on multiple checks (#22889)
Browse files Browse the repository at this point in the history
Since #22632, when a commit status has multiple checks, no check is
shown at all (hence no way to see the other checks).

This PR fixes this by always adding a tag with the
`.commit-statuses-trigger` to the DOM (the `.vm` is for vertical
alignment).

![2023-02-13-120528](https://user-images.githubusercontent.com/3864879/218441846-1a79c169-2efd-46bb-9e75-d8b45d7cc8e3.png)

---------

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
  • Loading branch information
oliverpool and lunny committed Feb 20, 2023
1 parent 1d64eaf commit 3596df5
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 47 deletions.
16 changes: 12 additions & 4 deletions templates/repo/commit_statuses.tmpl
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
{{if eq (len .Statuses) 1}}{{$status := index .Statuses 0}}{{if $status.TargetURL}}<a class="ui link commit-statuses-trigger gt-vm" href="{{$status.TargetURL}}">{{template "repo/commit_status" .Status}}</a>{{end}}{{end}}
<div class="ui commit-statuses-popup commit-statuses tippy-target">
<div class="ui relaxed list divided">
{{if .Statuses}}
{{if and (eq (len .Statuses) 1) .Status.TargetURL}}
<a class="gt-vm gt-tdn" data-tippy="commit-statuses" href="{{.Status.TargetURL}}">
{{template "repo/commit_status" .Status}}
</a>
{{else}}
<span class="gt-vm" data-tippy="commit-statuses" tabindex="0">
{{template "repo/commit_status" .Status}}
</span>
{{end}}
<div class="tippy-target ui relaxed list divided">
{{range .Statuses}}
<div class="ui item singular-status gt-df">
{{template "repo/commit_status" .}}
Expand All @@ -11,4 +19,4 @@
</div>
{{end}}
</div>
</div>
{{end}}
15 changes: 12 additions & 3 deletions tests/integration/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -630,8 +630,17 @@ func doAutoPRMerge(baseCtx *APITestContext, dstPath string) func(t *testing.T) {

commitID := path.Base(commitURL)

addCommitStatus := func(status api.CommitStatusState) func(*testing.T) {
return doAPICreateCommitStatus(ctx, commitID, api.CreateStatusOption{
State: status,
TargetURL: "http://test.ci/",
Description: "",
Context: "testci",
})
}

// Call API to add Pending status for commit
t.Run("CreateStatus", doAPICreateCommitStatus(ctx, commitID, api.CommitStatusPending))
t.Run("CreateStatus", addCommitStatus(api.CommitStatusPending))

// Cancel not existing auto merge
ctx.ExpectedCode = http.StatusNotFound
Expand Down Expand Up @@ -660,15 +669,15 @@ func doAutoPRMerge(baseCtx *APITestContext, dstPath string) func(t *testing.T) {
assert.False(t, pr.HasMerged)

// Call API to add Failure status for commit
t.Run("CreateStatus", doAPICreateCommitStatus(ctx, commitID, api.CommitStatusFailure))
t.Run("CreateStatus", addCommitStatus(api.CommitStatusFailure))

// Check pr status
pr, err = doAPIGetPullRequest(ctx, baseCtx.Username, baseCtx.Reponame, pr.Index)(t)
assert.NoError(t, err)
assert.False(t, pr.HasMerged)

// Call API to add Success status for commit
t.Run("CreateStatus", doAPICreateCommitStatus(ctx, commitID, api.CommitStatusSuccess))
t.Run("CreateStatus", addCommitStatus(api.CommitStatusSuccess))

// wait to let gitea merge stuff
time.Sleep(time.Second)
Expand Down
21 changes: 12 additions & 9 deletions tests/integration/pull_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,12 @@ func TestPullCreate_CommitStatus(t *testing.T) {
for _, status := range statusList {

// Call API to add status for commit
t.Run("CreateStatus", doAPICreateCommitStatus(testCtx, commitID, status))
t.Run("CreateStatus", doAPICreateCommitStatus(testCtx, commitID, api.CreateStatusOption{
State: status,
TargetURL: "http://test.ci/",
Description: "",
Context: "testci",
}))

req = NewRequestf(t, "GET", "/user1/repo1/pulls/1/commits")
resp = session.MakeRequest(t, req, http.StatusOK)
Expand All @@ -88,15 +93,13 @@ func TestPullCreate_CommitStatus(t *testing.T) {
})
}

func doAPICreateCommitStatus(ctx APITestContext, commitID string, status api.CommitStatusState) func(*testing.T) {
func doAPICreateCommitStatus(ctx APITestContext, commitID string, data api.CreateStatusOption) func(*testing.T) {
return func(t *testing.T) {
req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/statuses/%s?token=%s", ctx.Username, ctx.Reponame, commitID, ctx.Token),
api.CreateStatusOption{
State: status,
TargetURL: "http://test.ci/",
Description: "",
Context: "testci",
},
req := NewRequestWithJSON(
t,
http.MethodPost,
fmt.Sprintf("/api/v1/repos/%s/%s/statuses/%s?token=%s", ctx.Username, ctx.Reponame, commitID, ctx.Token),
data,
)
if ctx.ExpectedCode != 0 {
ctx.Session.MakeRequest(t, req, ctx.ExpectedCode)
Expand Down
58 changes: 54 additions & 4 deletions tests/integration/repo_commits_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,19 @@ func doTestRepoCommitWithStatus(t *testing.T, state string, classes ...string) {

// Call API to add status for commit
ctx := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeRepo)
t.Run("CreateStatus", doAPICreateCommitStatus(ctx, path.Base(commitURL), api.CommitStatusState(state)))
t.Run("CreateStatus", doAPICreateCommitStatus(ctx, path.Base(commitURL), api.CreateStatusOption{
State: api.CommitStatusState(state),
TargetURL: "http://test.ci/",
Description: "",
Context: "testci",
}))

req = NewRequest(t, "GET", "/user2/repo1/commits/branch/master")
resp = session.MakeRequest(t, req, http.StatusOK)

doc = NewHTMLParser(t, resp.Body)
// Check if commit status is displayed in message column
sel := doc.doc.Find("#commits-table tbody tr td.message a.commit-statuses-trigger .commit-status")
// Check if commit status is displayed in message column (.tippy-target to ignore the tippy trigger)
sel := doc.doc.Find("#commits-table tbody tr td.message .tippy-target .commit-status")
assert.Equal(t, 1, sel.Length())
for _, class := range classes {
assert.True(t, sel.HasClass(class))
Expand Down Expand Up @@ -145,11 +150,56 @@ func TestRepoCommitsStatusParallel(t *testing.T) {
go func(parentT *testing.T, i int) {
parentT.Run(fmt.Sprintf("ParallelCreateStatus_%d", i), func(t *testing.T) {
ctx := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeRepoStatus)
runBody := doAPICreateCommitStatus(ctx, path.Base(commitURL), api.CommitStatusState("pending"))
runBody := doAPICreateCommitStatus(ctx, path.Base(commitURL), api.CreateStatusOption{
State: api.CommitStatusPending,
TargetURL: "http://test.ci/",
Description: "",
Context: "testci",
})
runBody(t)
wg.Done()
})
}(t, i)
}
wg.Wait()
}

func TestRepoCommitsStatusMultiple(t *testing.T) {
defer tests.PrepareTestEnv(t)()

session := loginUser(t, "user2")

// Request repository commits page
req := NewRequest(t, "GET", "/user2/repo1/commits/branch/master")
resp := session.MakeRequest(t, req, http.StatusOK)

doc := NewHTMLParser(t, resp.Body)
// Get first commit URL
commitURL, exists := doc.doc.Find("#commits-table tbody tr td.sha a").Attr("href")
assert.True(t, exists)
assert.NotEmpty(t, commitURL)

// Call API to add status for commit
ctx := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeRepo)
t.Run("CreateStatus", doAPICreateCommitStatus(ctx, path.Base(commitURL), api.CreateStatusOption{
State: api.CommitStatusSuccess,
TargetURL: "http://test.ci/",
Description: "",
Context: "testci",
}))

t.Run("CreateStatus", doAPICreateCommitStatus(ctx, path.Base(commitURL), api.CreateStatusOption{
State: api.CommitStatusSuccess,
TargetURL: "http://test.ci/",
Description: "",
Context: "other_context",
}))

req = NewRequest(t, "GET", "/user2/repo1/commits/branch/master")
resp = session.MakeRequest(t, req, http.StatusOK)

doc = NewHTMLParser(t, resp.Body)
// Check that the data-tippy="commit-statuses" (for trigger) and commit-status (svg) are present
sel := doc.doc.Find("#commits-table tbody tr td.message [data-tippy=\"commit-statuses\"] .commit-status")
assert.Equal(t, 1, sel.Length())
}
2 changes: 1 addition & 1 deletion web_src/js/features/repo-commit.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export function initRepoCommitLastCommitLoader() {
}

export function initCommitStatuses() {
$('.commit-statuses-trigger').each(function () {
$('[data-tippy="commit-statuses"]').each(function () {
const top = $('.repository.file.list').length > 0 || $('.repository.diff').length > 0;

createTippy(this, {
Expand Down
3 changes: 1 addition & 2 deletions web_src/less/_base.less
Original file line number Diff line number Diff line change
Expand Up @@ -340,8 +340,7 @@ a.label,
.ui.search .results a,
.ui .menu a,
.ui.cards a.card,
.issue-keyword a,
a.commit-statuses-trigger {
.issue-keyword a {
text-decoration: none !important;
}

Expand Down
24 changes: 0 additions & 24 deletions web_src/less/_repository.less
Original file line number Diff line number Diff line change
@@ -1,28 +1,4 @@
.repository {
.popup.commit-statuses {
// we had better limit the max size of the popup, and add scroll bars if the content size is too large.
// otherwise some part of the popup will be hidden by viewport boundary
max-height: 45vh;
max-width: 60vw;

&.ui.right {
// Override `.ui.attached.header .right:not(.dropdown) height: 30px;` which would otherwise lead to
// the status popup box having its height fixed at 30px. See https://github.com/go-gitea/gitea/issues/18498
height: auto;
}

overflow: auto;
padding: 0;

.list {
padding: .8em; // to make the scrollbar align to the border, we move the padding from outer `.popup` to this inside `.list`

> .item {
line-height: 2;
}
}
}

.repo-header {
.ui.compact.menu {
margin-left: 1rem;
Expand Down

0 comments on commit 3596df5

Please sign in to comment.