diff options
Diffstat (limited to 'models/git')
| -rw-r--r-- | models/git/branch.go | 2 | ||||
| -rw-r--r-- | models/git/commit_status.go | 8 | ||||
| -rw-r--r-- | models/git/commit_status_summary.go | 6 |
3 files changed, 13 insertions, 3 deletions
diff --git a/models/git/branch.go b/models/git/branch.go index beeb7c0689..07c94a8ba5 100644 --- a/models/git/branch.go +++ b/models/git/branch.go @@ -487,7 +487,7 @@ func FindRecentlyPushedNewBranches(ctx context.Context, doer *user_model.User, o ForkFrom: opts.BaseRepo.ID, Archived: optional.Some(false), } - repoCond := repo_model.SearchRepositoryCondition(&repoOpts).And(repo_model.AccessibleRepositoryCondition(doer, unit.TypeCode)) + repoCond := repo_model.SearchRepositoryCondition(repoOpts).And(repo_model.AccessibleRepositoryCondition(doer, unit.TypeCode)) if opts.Repo.ID == opts.BaseRepo.ID { // should also include the base repo's branches repoCond = repoCond.Or(builder.Eq{"id": opts.BaseRepo.ID}) diff --git a/models/git/commit_status.go b/models/git/commit_status.go index 0e4e8215b0..2e765391b8 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -230,18 +230,24 @@ func (status *CommitStatus) HideActionsURL(ctx context.Context) { // CalcCommitStatus returns commit status state via some status, the commit statues should order by id desc func CalcCommitStatus(statuses []*CommitStatus) *CommitStatus { + // This function is widely used, but it is not quite right. + // Ideally it should return something like "CommitStatusSummary" with properly aggregated state. + // GitHub's behavior: if all statuses are "skipped", GitHub will return "success" as the combined status. var lastStatus *CommitStatus state := api.CommitStatusSuccess for _, status := range statuses { - if status.State.NoBetterThan(state) { + if state == status.State || status.State.HasHigherPriorityThan(state) { state = status.State lastStatus = status } } if lastStatus == nil { if len(statuses) > 0 { + // FIXME: a bad case: Gitea just returns the first commit status, its status is "skipped" in this case. lastStatus = statuses[0] } else { + // FIXME: another bad case: if the "statuses" slice is empty, the returned value is an invalid CommitStatus, all its fields are empty. + // Frontend code (tmpl&vue) sometimes depend on the empty fields to skip rendering commit status elements (need to double check in the future) lastStatus = &CommitStatus{} } } diff --git a/models/git/commit_status_summary.go b/models/git/commit_status_summary.go index a3f440fd86..774e49bb98 100644 --- a/models/git/commit_status_summary.go +++ b/models/git/commit_status_summary.go @@ -59,7 +59,11 @@ func UpdateCommitStatusSummary(ctx context.Context, repoID int64, sha string) er if err != nil { return err } - state := CalcCommitStatus(commitStatuses) + // it guarantees that commitStatuses is not empty because this function is always called after a commit status is created + if len(commitStatuses) == 0 { + setting.PanicInDevOrTesting("no commit statuses found for repo %d and sha %s", repoID, sha) + } + state := CalcCommitStatus(commitStatuses) // non-empty commitStatuses is guaranteed // mysql will return 0 when update a record which state hasn't been changed which behaviour is different from other database, // so we need to use insert in on duplicate if setting.Database.Type.IsMySQL() { |