Fix wrong commit status in web ui

[yp05327]

Before:

After:

There's a bug in the recent logic, CalcCommitStatus will always return the first item of statuses or error status, because state is defined with default value which should be CommitStatusSuccess

Then

if status.State.NoBetterThan(state) {

this if will always return false unless status.State = CommitStatusError which makes no sense.
So lastStatus will always be nil or error status.
Then we will always return the first item of statuses here or only return error status, and this is why in the first picture the commit status is Success but not Failure.

gitea/models/git/commit_status.go

Lines 204 to 211 in af1ffbc

	if lastStatus == nil {
	if len(statuses) > 0 {
	lastStatus = statuses[0]
	} else {
	lastStatus = &CommitStatus{}
	}
	}
	return lastStatus

[yp05327]

It seems that this logic problem comes from #25839
In old code, CalcCommitStatus will return true when the input is an empty string(default value).

default:
    return css2 != CommitStatusError && css2 != CommitStatusFailure && css2 != CommitStatusWarning && css2 != CommitStatusPending

[silverwind]

Possible to add a test that verifies that "ok, fail" checks result in "fail"?

[yp05327]

Possible to add a test that verifies that "ok, fail" checks result in "fail"?

Maybe a unit test for CalcCommitStatus is enough?

[wxiaoguang]

It could be simplified to:

	var lastStatus *CommitStatus
	if lastStatus == nil || status.State.NoBetterThan(lastStatus.state) {
		lastStatus = status
	}

[CaiCandong]

It seems that this logic problem comes from #25839

My fault, the status.State.NoBetterThan I wrote only handles CommitStatusError, CommitStatusFailure CommitStatusPending, CommitStatusSuccess.
A better way to work around this bug would be to make the NoBetterThan function more robust? but It's a bit strange that to set the empty string "" is best.
The function is used elsewhere, and there seems to be a logic error in this place as well:

gitea/services/convert/status.go

Lines 41 to 54 in 6598d02

	retStatus := &api.CombinedStatus{
	SHA: statuses[0].SHA,
	TotalCount: len(statuses),
	Repository: repo,
	URL: "",
	}
	retStatus.Statuses = make([]*api.CommitStatus, 0, len(statuses))
	for _, status := range statuses {
	retStatus.Statuses = append(retStatus.Statuses, ToCommitStatus(ctx, status))
	if status.State.NoBetterThan(retStatus.State) {
	retStatus.State = status.State
	}
	}

This place looks right.

gitea/services/pull/commit_status.go

Lines 38 to 49 in 6598d02

	var targetStatus structs.CommitStatusState
	for _, gp := range requiredContextsGlob {
	if gp.Match(commitStatus.Context) {
	targetStatus = commitStatus.State
	matchedCount++
	break
	}
	}
	if targetStatus != "" && targetStatus.NoBetterThan(returnedStatus) {
	returnedStatus = targetStatus
	}

[wolfogre]

I believe this PR is good enough to merge, @CaiCandong please help to improve if you have any ideas.