mirror of
				https://github.com/go-gitea/gitea.git
				synced 2025-10-29 10:57:44 +09:00 
			
		
		
		
	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).  --------- Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
This commit is contained in:
		| @@ -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}} | {{if .Statuses}} | ||||||
| <div class="ui commit-statuses-popup commit-statuses tippy-target"> | 	{{if and (eq (len .Statuses) 1) .Status.TargetURL}} | ||||||
| 	<div class="ui relaxed list divided"> | 		<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}} | 		{{range .Statuses}} | ||||||
| 			<div class="ui item singular-status gt-df"> | 			<div class="ui item singular-status gt-df"> | ||||||
| 				{{template "repo/commit_status" .}} | 				{{template "repo/commit_status" .}} | ||||||
| @@ -11,4 +19,4 @@ | |||||||
| 			</div> | 			</div> | ||||||
| 		{{end}} | 		{{end}} | ||||||
| 	</div> | 	</div> | ||||||
| </div> | {{end}} | ||||||
|   | |||||||
| @@ -630,8 +630,17 @@ func doAutoPRMerge(baseCtx *APITestContext, dstPath string) func(t *testing.T) { | |||||||
|  |  | ||||||
| 		commitID := path.Base(commitURL) | 		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 | 		// 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 | 		// Cancel not existing auto merge | ||||||
| 		ctx.ExpectedCode = http.StatusNotFound | 		ctx.ExpectedCode = http.StatusNotFound | ||||||
| @@ -660,7 +669,7 @@ func doAutoPRMerge(baseCtx *APITestContext, dstPath string) func(t *testing.T) { | |||||||
| 		assert.False(t, pr.HasMerged) | 		assert.False(t, pr.HasMerged) | ||||||
|  |  | ||||||
| 		// Call API to add Failure status for commit | 		// 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 | 		// Check pr status | ||||||
| 		pr, err = doAPIGetPullRequest(ctx, baseCtx.Username, baseCtx.Reponame, pr.Index)(t) | 		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) | 		assert.False(t, pr.HasMerged) | ||||||
|  |  | ||||||
| 		// Call API to add Success status for commit | 		// 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 | 		// wait to let gitea merge stuff | ||||||
| 		time.Sleep(time.Second) | 		time.Sleep(time.Second) | ||||||
|   | |||||||
| @@ -70,7 +70,12 @@ func TestPullCreate_CommitStatus(t *testing.T) { | |||||||
| 		for _, status := range statusList { | 		for _, status := range statusList { | ||||||
|  |  | ||||||
| 			// Call API to add status for commit | 			// 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") | 			req = NewRequestf(t, "GET", "/user1/repo1/pulls/1/commits") | ||||||
| 			resp = session.MakeRequest(t, req, http.StatusOK) | 			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) { | 	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), | 		req := NewRequestWithJSON( | ||||||
| 			api.CreateStatusOption{ | 			t, | ||||||
| 				State:       status, | 			http.MethodPost, | ||||||
| 				TargetURL:   "http://test.ci/", | 			fmt.Sprintf("/api/v1/repos/%s/%s/statuses/%s?token=%s", ctx.Username, ctx.Reponame, commitID, ctx.Token), | ||||||
| 				Description: "", | 			data, | ||||||
| 				Context:     "testci", |  | ||||||
| 			}, |  | ||||||
| 		) | 		) | ||||||
| 		if ctx.ExpectedCode != 0 { | 		if ctx.ExpectedCode != 0 { | ||||||
| 			ctx.Session.MakeRequest(t, req, ctx.ExpectedCode) | 			ctx.Session.MakeRequest(t, req, ctx.ExpectedCode) | ||||||
|   | |||||||
| @@ -52,14 +52,19 @@ func doTestRepoCommitWithStatus(t *testing.T, state string, classes ...string) { | |||||||
|  |  | ||||||
| 	// Call API to add status for commit | 	// Call API to add status for commit | ||||||
| 	ctx := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeRepo) | 	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") | 	req = NewRequest(t, "GET", "/user2/repo1/commits/branch/master") | ||||||
| 	resp = session.MakeRequest(t, req, http.StatusOK) | 	resp = session.MakeRequest(t, req, http.StatusOK) | ||||||
|  |  | ||||||
| 	doc = NewHTMLParser(t, resp.Body) | 	doc = NewHTMLParser(t, resp.Body) | ||||||
| 	// Check if commit status is displayed in message column | 	// 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 a.commit-statuses-trigger .commit-status") | 	sel := doc.doc.Find("#commits-table tbody tr td.message .tippy-target .commit-status") | ||||||
| 	assert.Equal(t, 1, sel.Length()) | 	assert.Equal(t, 1, sel.Length()) | ||||||
| 	for _, class := range classes { | 	for _, class := range classes { | ||||||
| 		assert.True(t, sel.HasClass(class)) | 		assert.True(t, sel.HasClass(class)) | ||||||
| @@ -145,7 +150,12 @@ func TestRepoCommitsStatusParallel(t *testing.T) { | |||||||
| 		go func(parentT *testing.T, i int) { | 		go func(parentT *testing.T, i int) { | ||||||
| 			parentT.Run(fmt.Sprintf("ParallelCreateStatus_%d", i), func(t *testing.T) { | 			parentT.Run(fmt.Sprintf("ParallelCreateStatus_%d", i), func(t *testing.T) { | ||||||
| 				ctx := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeRepoStatus) | 				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) | 				runBody(t) | ||||||
| 				wg.Done() | 				wg.Done() | ||||||
| 			}) | 			}) | ||||||
| @@ -153,3 +163,43 @@ func TestRepoCommitsStatusParallel(t *testing.T) { | |||||||
| 	} | 	} | ||||||
| 	wg.Wait() | 	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()) | ||||||
|  | } | ||||||
|   | |||||||
| @@ -58,7 +58,7 @@ export function initRepoCommitLastCommitLoader() { | |||||||
| } | } | ||||||
|  |  | ||||||
| export function initCommitStatuses() { | 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; |     const top = $('.repository.file.list').length > 0 || $('.repository.diff').length > 0; | ||||||
|  |  | ||||||
|     createTippy(this, { |     createTippy(this, { | ||||||
|   | |||||||
| @@ -340,8 +340,7 @@ a.label, | |||||||
| .ui.search .results a, | .ui.search .results a, | ||||||
| .ui .menu a, | .ui .menu a, | ||||||
| .ui.cards a.card, | .ui.cards a.card, | ||||||
| .issue-keyword a, | .issue-keyword a { | ||||||
| a.commit-statuses-trigger { |  | ||||||
|   text-decoration: none !important; |   text-decoration: none !important; | ||||||
| } | } | ||||||
|  |  | ||||||
|   | |||||||
| @@ -1,28 +1,4 @@ | |||||||
| .repository { | .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 { |   .repo-header { | ||||||
|     .ui.compact.menu { |     .ui.compact.menu { | ||||||
|       margin-left: 1rem; |       margin-left: 1rem; | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user