From 3596df52c09831f7f39f8416264ff267954f35a0 Mon Sep 17 00:00:00 2001 From: oliverpool <3864879+oliverpool@users.noreply.github.com> Date: Mon, 20 Feb 2023 09:43:04 +0100 Subject: [PATCH] Fix hidden commit status on multiple checks (#22889) 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 --- templates/repo/commit_statuses.tmpl | 16 +++++-- tests/integration/git_test.go | 15 +++++-- tests/integration/pull_status_test.go | 21 ++++++---- tests/integration/repo_commits_test.go | 58 ++++++++++++++++++++++++-- web_src/js/features/repo-commit.js | 2 +- web_src/less/_base.less | 3 +- web_src/less/_repository.less | 24 ----------- 7 files changed, 92 insertions(+), 47 deletions(-) diff --git a/templates/repo/commit_statuses.tmpl b/templates/repo/commit_statuses.tmpl index 8250a8581..8858fb840 100644 --- a/templates/repo/commit_statuses.tmpl +++ b/templates/repo/commit_statuses.tmpl @@ -1,6 +1,14 @@ -{{if eq (len .Statuses) 1}}{{$status := index .Statuses 0}}{{if $status.TargetURL}}{{template "repo/commit_status" .Status}}{{end}}{{end}} -
-
+{{if .Statuses}} + {{if and (eq (len .Statuses) 1) .Status.TargetURL}} + + {{template "repo/commit_status" .Status}} + + {{else}} + + {{template "repo/commit_status" .Status}} + + {{end}} +
{{range .Statuses}}
{{template "repo/commit_status" .}} @@ -11,4 +19,4 @@
{{end}}
-
+{{end}} diff --git a/tests/integration/git_test.go b/tests/integration/git_test.go index 420a8676b..d21f3994a 100644 --- a/tests/integration/git_test.go +++ b/tests/integration/git_test.go @@ -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 @@ -660,7 +669,7 @@ 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) @@ -668,7 +677,7 @@ func doAutoPRMerge(baseCtx *APITestContext, dstPath string) func(t *testing.T) { 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) diff --git a/tests/integration/pull_status_test.go b/tests/integration/pull_status_test.go index e60d17edc..736d1ee4f 100644 --- a/tests/integration/pull_status_test.go +++ b/tests/integration/pull_status_test.go @@ -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) @@ -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) diff --git a/tests/integration/repo_commits_test.go b/tests/integration/repo_commits_test.go index cbd83c6de..e74e3867f 100644 --- a/tests/integration/repo_commits_test.go +++ b/tests/integration/repo_commits_test.go @@ -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)) @@ -145,7 +150,12 @@ 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() }) @@ -153,3 +163,43 @@ func TestRepoCommitsStatusParallel(t *testing.T) { } 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()) +} diff --git a/web_src/js/features/repo-commit.js b/web_src/js/features/repo-commit.js index e2eef3ee5..d22aa8e98 100644 --- a/web_src/js/features/repo-commit.js +++ b/web_src/js/features/repo-commit.js @@ -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, { diff --git a/web_src/less/_base.less b/web_src/less/_base.less index fae29d0d6..fc04df4f9 100644 --- a/web_src/less/_base.less +++ b/web_src/less/_base.less @@ -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; } diff --git a/web_src/less/_repository.less b/web_src/less/_repository.less index f48933571..3bec5f58f 100644 --- a/web_src/less/_repository.less +++ b/web_src/less/_repository.less @@ -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;