From 840830b655a65c0763e3fd4bd0ced9256d2081a5 Mon Sep 17 00:00:00 2001 From: caicandong <50507092+CaiCandong@users.noreply.github.com> Date: Fri, 21 Jul 2023 16:24:36 +0800 Subject: [PATCH] Remove commit status running and warning to align GitHub (#25839) Fix #25776. Close #25826. In the discussion of #25776, @wolfogre's suggestion was to remove the commit status of `running` and `warning` to keep it consistent with github. references: - https://docs.github.com/en/rest/commits/statuses?apiVersion=2022-11-28#about-commit-statuses ## :warning: BREAKING :warning: So the commit status of Gitea will be consistent with GitHub, only `pending`, `success`, `error` and `failure`, while `warning` and `running` are not supported anymore. --------- Co-authored-by: Jason Song --- models/git/commit_status_test.go | 4 ---- models/migrations/migrations.go | 2 ++ models/migrations/v1_21/v266.go | 26 ++++++++++++++++++++++++++ modules/structs/commit_status.go | 26 ++++++-------------------- services/actions/commit_status.go | 8 ++------ services/convert/status.go | 8 ++++++++ templates/repo/commit_status.tmpl | 5 +---- tests/integration/pull_status_test.go | 2 -- tests/integration/repo_commits_test.go | 8 -------- 9 files changed, 45 insertions(+), 44 deletions(-) create mode 100644 models/migrations/v1_21/v266.go diff --git a/models/git/commit_status_test.go b/models/git/commit_status_test.go index 2197433b3e..a86941a0fe 100644 --- a/models/git/commit_status_test.go +++ b/models/git/commit_status_test.go @@ -31,10 +31,6 @@ func TestGetCommitStatuses(t *testing.T) { assert.Equal(t, structs.CommitStatusPending, statuses[0].State) assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[0].APIURL(db.DefaultContext)) - assert.Equal(t, "cov/awesomeness", statuses[1].Context) - assert.Equal(t, structs.CommitStatusWarning, statuses[1].State) - assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[1].APIURL(db.DefaultContext)) - assert.Equal(t, "cov/awesomeness", statuses[2].Context) assert.Equal(t, structs.CommitStatusSuccess, statuses[2].State) assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[2].APIURL(db.DefaultContext)) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 9596b99c12..6599cb9cda 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -513,6 +513,8 @@ var migrations = []Migration{ NewMigration("Add branch table", v1_21.AddBranchTable), // v265 -> v266 NewMigration("Alter Actions Artifact table", v1_21.AlterActionArtifactTable), + // v266 -> v267 + NewMigration("Reduce commit status", v1_21.ReduceCommitStatus), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_21/v266.go b/models/migrations/v1_21/v266.go new file mode 100644 index 0000000000..df85286c89 --- /dev/null +++ b/models/migrations/v1_21/v266.go @@ -0,0 +1,26 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_21 //nolint + +import ( + "xorm.io/xorm" +) + +func ReduceCommitStatus(x *xorm.Engine) error { + sess := x.NewSession() + defer sess.Close() + + if err := sess.Begin(); err != nil { + return err + } + + if _, err := sess.Exec(`UPDATE commit_status SET state='pending' WHERE state='running'`); err != nil { + return err + } + if _, err := sess.Exec(`UPDATE commit_status SET state='failure' WHERE state='warning'`); err != nil { + return err + } + + return sess.Commit() +} diff --git a/modules/structs/commit_status.go b/modules/structs/commit_status.go index 7e3b629b7a..ff31f2d2ac 100644 --- a/modules/structs/commit_status.go +++ b/modules/structs/commit_status.go @@ -16,26 +16,17 @@ const ( CommitStatusError CommitStatusState = "error" // CommitStatusFailure is for when the CommitStatus is Failure CommitStatusFailure CommitStatusState = "failure" - // CommitStatusWarning is for when the CommitStatus is Warning - CommitStatusWarning CommitStatusState = "warning" - // CommitStatusRunning is for when the CommitStatus is Running - CommitStatusRunning CommitStatusState = "running" ) // NoBetterThan returns true if this State is no better than the given State func (css CommitStatusState) NoBetterThan(css2 CommitStatusState) bool { - switch css { - case CommitStatusError: - return true - case CommitStatusFailure: - return css2 != CommitStatusError - case CommitStatusWarning: - return css2 != CommitStatusError && css2 != CommitStatusFailure - case CommitStatusPending: - return css2 != CommitStatusError && css2 != CommitStatusFailure && css2 != CommitStatusWarning - default: - return css2 != CommitStatusError && css2 != CommitStatusFailure && css2 != CommitStatusWarning && css2 != CommitStatusPending + commitStatusPriorities := map[CommitStatusState]int{ + CommitStatusError: 0, + CommitStatusFailure: 1, + CommitStatusPending: 2, + CommitStatusSuccess: 3, } + return commitStatusPriorities[css] <= commitStatusPriorities[css2] } // IsPending represents if commit status state is pending @@ -57,8 +48,3 @@ func (css CommitStatusState) IsError() bool { func (css CommitStatusState) IsFailure() bool { return css == CommitStatusFailure } - -// IsWarning represents if commit status state is warning -func (css CommitStatusState) IsWarning() bool { - return css == CommitStatusWarning -} diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go index 6114f2b443..925d0a3396 100644 --- a/services/actions/commit_status.go +++ b/services/actions/commit_status.go @@ -137,14 +137,10 @@ func toCommitStatus(status actions_model.Status) api.CommitStatusState { switch status { case actions_model.StatusSuccess, actions_model.StatusSkipped: return api.CommitStatusSuccess - case actions_model.StatusFailure: + case actions_model.StatusFailure, actions_model.StatusCancelled: return api.CommitStatusFailure - case actions_model.StatusCancelled: - return api.CommitStatusWarning - case actions_model.StatusWaiting, actions_model.StatusBlocked: + case actions_model.StatusWaiting, actions_model.StatusBlocked, actions_model.StatusRunning: return api.CommitStatusPending - case actions_model.StatusRunning: - return api.CommitStatusRunning default: return api.CommitStatusError } diff --git a/services/convert/status.go b/services/convert/status.go index b8c11ab630..c7b6450e27 100644 --- a/services/convert/status.go +++ b/services/convert/status.go @@ -52,6 +52,14 @@ func ToCombinedStatus(ctx context.Context, statuses []*git_model.CommitStatus, r retStatus.State = status.State } } + // According to https://docs.github.com/en/rest/commits/statuses?apiVersion=2022-11-28#get-the-combined-status-for-a-specific-reference + // > Additionally, a combined state is returned. The state is one of: + // > failure if any of the contexts report as error or failure + // > pending if there are no statuses or a context is pending + // > success if the latest status for all contexts is success + if retStatus.State.IsError() { + retStatus.State = api.CommitStatusFailure + } return retStatus } diff --git a/templates/repo/commit_status.tmpl b/templates/repo/commit_status.tmpl index 6a01b9f342..2dfd757ee1 100644 --- a/templates/repo/commit_status.tmpl +++ b/templates/repo/commit_status.tmpl @@ -1,4 +1,4 @@ -{{if or (eq .State "pending") (eq .State "running")}} +{{if eq .State "pending"}} {{svg "octicon-dot-fill" 18 "commit-status icon text yellow"}} {{end}} {{if eq .State "success"}} @@ -10,6 +10,3 @@ {{if eq .State "failure"}} {{svg "octicon-x" 18 "commit-status icon text red"}} {{end}} -{{if eq .State "warning"}} - {{svg "gitea-exclamation" 18 "commit-status icon text yellow"}} -{{end}} diff --git a/tests/integration/pull_status_test.go b/tests/integration/pull_status_test.go index 6a6fd2e859..7c1f8c701e 100644 --- a/tests/integration/pull_status_test.go +++ b/tests/integration/pull_status_test.go @@ -52,7 +52,6 @@ func TestPullCreate_CommitStatus(t *testing.T) { api.CommitStatusPending, api.CommitStatusError, api.CommitStatusFailure, - api.CommitStatusWarning, api.CommitStatusSuccess, } @@ -61,7 +60,6 @@ func TestPullCreate_CommitStatus(t *testing.T) { api.CommitStatusSuccess: "octicon-check", api.CommitStatusError: "gitea-exclamation", api.CommitStatusFailure: "octicon-x", - api.CommitStatusWarning: "gitea-exclamation", } testCtx := NewAPITestContext(t, "user1", "repo1", auth_model.AccessTokenScopeWriteRepository) diff --git a/tests/integration/repo_commits_test.go b/tests/integration/repo_commits_test.go index 9a9836a16f..789b5f7cc3 100644 --- a/tests/integration/repo_commits_test.go +++ b/tests/integration/repo_commits_test.go @@ -125,14 +125,6 @@ func TestRepoCommitsWithStatusFailure(t *testing.T) { doTestRepoCommitWithStatus(t, "failure", "octicon-x", "red") } -func TestRepoCommitsWithStatusWarning(t *testing.T) { - doTestRepoCommitWithStatus(t, "warning", "gitea-exclamation", "yellow") -} - -func TestRepoCommitsWithStatusRunning(t *testing.T) { - doTestRepoCommitWithStatus(t, "running", "octicon-dot-fill", "yellow") -} - func TestRepoCommitsStatusParallel(t *testing.T) { defer tests.PrepareTestEnv(t)()