Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix hidden commit status on multiple checks #22889

Merged
merged 14 commits into from
Feb 20, 2023
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}}
oliverpool marked this conversation as resolved.
Show resolved Hide resolved
<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)
oliverpool marked this conversation as resolved.
Show resolved Hide resolved
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 () {
oliverpool marked this conversation as resolved.
Show resolved Hide resolved
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 {
oliverpool marked this conversation as resolved.
Show resolved Hide resolved
.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
oliverpool marked this conversation as resolved.
Show resolved Hide resolved
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