Merge pull request '[v1.22/gitea] week 2024-18 cherry pick v7.0 (take 2)' (#3580) from earl-warren/forgejo:wip-v7.0-gitea-cherry-pick into v7.0/forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/3580
Reviewed-by: twenty-panda <twenty-panda@noreply.codeberg.org>

models/git/commit_status.go

@@ -395,36 +395,16 @@ func GetLatestCommitStatusForRepoCommitIDs(ctx context.Context, repoID int64, co
 
 // FindRepoRecentCommitStatusContexts returns repository's recent commit status contexts
 func FindRepoRecentCommitStatusContexts(ctx context.Context, repoID int64, before time.Duration) ([]string, error) {
-	type result struct {
-		Index int64
-		SHA   string
-	}
-	getBase := func() *xorm.Session {
-		return db.GetEngine(ctx).Table(&CommitStatus{}).Where("repo_id = ?", repoID)
-	}
-
 	start := timeutil.TimeStampNow().AddDuration(-before)
-	results := make([]result, 0, 10)
 
-	sess := getBase().And("updated_unix >= ?", start).
+	var contexts []string
-		Select("max( `index` ) as `index`, sha").
+	if err := db.GetEngine(ctx).Table("commit_status").
-		GroupBy("context_hash, sha").OrderBy("max( `index` ) desc")
+		Where("repo_id = ?", repoID).And("updated_unix >= ?", start).
-
+		Cols("context").Distinct().Find(&contexts); err != nil {
-	err := sess.Find(&results)
-	if err != nil {
 		return nil, err
 	}
 
-	contexts := make([]string, 0, len(results))
+	return contexts, nil
-	if len(results) == 0 {
-		return contexts, nil
-	}
-
-	conds := make([]builder.Cond, 0, len(results))
-	for _, result := range results {
-		conds = append(conds, builder.Eq{"`index`": result.Index, "sha": result.SHA})
-	}
-	return contexts, getBase().And(builder.Or(conds...)).Select("context").Find(&contexts)
 }
 
 // NewCommitStatusOptions holds options for creating a CommitStatus

models/git/commit_status_test.go

@@ -5,11 +5,15 @@ package git_test
 
 import (
 	"testing"
+	"time"
 
 	"code.gitea.io/gitea/models/db"
 	git_model "code.gitea.io/gitea/models/git"
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unittest"
+	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/git"
+	"code.gitea.io/gitea/modules/gitrepo"
 	"code.gitea.io/gitea/modules/structs"
 
 	"github.com/stretchr/testify/assert"
@@ -183,3 +187,55 @@ func Test_CalcCommitStatus(t *testing.T) {
 		assert.Equal(t, kase.expected, git_model.CalcCommitStatus(kase.statuses))
 	}
 }
+
+func TestFindRepoRecentCommitStatusContexts(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
+	user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+	gitRepo, err := gitrepo.OpenRepository(git.DefaultContext, repo2)
+	assert.NoError(t, err)
+	defer gitRepo.Close()
+
+	commit, err := gitRepo.GetBranchCommit(repo2.DefaultBranch)
+	assert.NoError(t, err)
+
+	defer func() {
+		_, err := db.DeleteByBean(db.DefaultContext, &git_model.CommitStatus{
+			RepoID:    repo2.ID,
+			CreatorID: user2.ID,
+			SHA:       commit.ID.String(),
+		})
+		assert.NoError(t, err)
+	}()
+
+	err = git_model.NewCommitStatus(db.DefaultContext, git_model.NewCommitStatusOptions{
+		Repo:    repo2,
+		Creator: user2,
+		SHA:     commit.ID,
+		CommitStatus: &git_model.CommitStatus{
+			State:     structs.CommitStatusFailure,
+			TargetURL: "https://example.com/tests/",
+			Context:   "compliance/lint-backend",
+		},
+	})
+	assert.NoError(t, err)
+
+	err = git_model.NewCommitStatus(db.DefaultContext, git_model.NewCommitStatusOptions{
+		Repo:    repo2,
+		Creator: user2,
+		SHA:     commit.ID,
+		CommitStatus: &git_model.CommitStatus{
+			State:     structs.CommitStatusSuccess,
+			TargetURL: "https://example.com/tests/",
+			Context:   "compliance/lint-backend",
+		},
+	})
+	assert.NoError(t, err)
+
+	contexts, err := git_model.FindRepoRecentCommitStatusContexts(db.DefaultContext, repo2.ID, time.Hour)
+	assert.NoError(t, err)
+	if assert.Len(t, contexts, 1) {
+		assert.Equal(t, "compliance/lint-backend", contexts[0])
+	}
+}

models/repo/user_repo.go

@@ -95,7 +95,10 @@ func GetRepoAssignees(ctx context.Context, repo *Repository) (_ []*user_model.Us
 	// and just waste 1 unit is cheaper than re-allocate memory once.
 	users := make([]*user_model.User, 0, len(uniqueUserIDs)+1)
 	if len(userIDs) > 0 {
-		if err = e.In("id", uniqueUserIDs.Values()).OrderBy(user_model.GetOrderByName()).Find(&users); err != nil {
+		if err = e.In("id", uniqueUserIDs.Values()).
+			Where(builder.Eq{"`user`.is_active": true}).
+			OrderBy(user_model.GetOrderByName()).
+			Find(&users); err != nil {
 			return nil, err
 		}
 	}
@@ -117,7 +120,8 @@ func GetReviewers(ctx context.Context, repo *Repository, doerID, posterID int64)
 		return nil, err
 	}
 
-	cond := builder.And(builder.Neq{"`user`.id": posterID})
+	cond := builder.And(builder.Neq{"`user`.id": posterID}).
+		And(builder.Eq{"`user`.is_active": true})
 
 	if repo.IsPrivate || repo.Owner.Visibility == api.VisibleTypePrivate {
 		// This a private repository:

models/repo/user_repo_test.go

@@ -26,10 +26,17 @@ func TestRepoAssignees(t *testing.T) {
 	repo21 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 21})
 	users, err = repo_model.GetRepoAssignees(db.DefaultContext, repo21)
 	assert.NoError(t, err)
-	assert.Len(t, users, 3)
+	if assert.Len(t, users, 3) {
-	assert.Equal(t, users[0].ID, int64(15))
+		assert.ElementsMatch(t, []int64{15, 16, 18}, []int64{users[0].ID, users[1].ID, users[2].ID})
-	assert.Equal(t, users[1].ID, int64(18))
+	}
-	assert.Equal(t, users[2].ID, int64(16))
+
+	// do not return deactivated users
+	assert.NoError(t, user_model.UpdateUserCols(db.DefaultContext, &user_model.User{ID: 15, IsActive: false}, "is_active"))
+	users, err = repo_model.GetRepoAssignees(db.DefaultContext, repo21)
+	assert.NoError(t, err)
+	if assert.Len(t, users, 2) {
+		assert.NotContains(t, []int64{users[0].ID, users[1].ID}, 15)
+	}
 }
 
 func TestRepoGetReviewers(t *testing.T) {
@@ -41,17 +48,19 @@ func TestRepoGetReviewers(t *testing.T) {
 	ctx := db.DefaultContext
 	reviewers, err := repo_model.GetReviewers(ctx, repo1, 2, 2)
 	assert.NoError(t, err)
-	assert.Len(t, reviewers, 4)
+	if assert.Len(t, reviewers, 3) {
+		assert.ElementsMatch(t, []int64{1, 4, 11}, []int64{reviewers[0].ID, reviewers[1].ID, reviewers[2].ID})
+	}
 
 	// should include doer if doer is not PR poster.
 	reviewers, err = repo_model.GetReviewers(ctx, repo1, 11, 2)
 	assert.NoError(t, err)
-	assert.Len(t, reviewers, 4)
+	assert.Len(t, reviewers, 3)
 
 	// should not include PR poster, if PR poster would be otherwise eligible
 	reviewers, err = repo_model.GetReviewers(ctx, repo1, 11, 4)
 	assert.NoError(t, err)
-	assert.Len(t, reviewers, 3)
+	assert.Len(t, reviewers, 2)
 
 	// test private user repo
 	repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})

routers/api/actions/artifacts.go

@@ -467,14 +467,15 @@ func (ar artifactRoutes) downloadArtifact(ctx *ArtifactContext) {
 		log.Error("Error getting artifact: %v", err)
 		ctx.Error(http.StatusInternalServerError, err.Error())
 		return
-	} else if !exist {
+	}
+	if !exist {
 		log.Error("artifact with ID %d does not exist", artifactID)
 		ctx.Error(http.StatusNotFound, fmt.Sprintf("artifact with ID %d does not exist", artifactID))
 		return
 	}
 	if artifact.RunID != runID {
-		log.Error("Error dismatch runID and artifactID, task: %v, artifact: %v", runID, artifactID)
+		log.Error("Error mismatch runID and artifactID, task: %v, artifact: %v", runID, artifactID)
-		ctx.Error(http.StatusBadRequest, err.Error())
+		ctx.Error(http.StatusBadRequest)
 		return
 	}
 

routers/web/repo/actions/view.go

@@ -552,7 +552,7 @@ func getRunJobs(ctx *context_module.Context, runIndex, jobIndex int64) (*actions
 		return nil, nil
 	}
 	if len(jobs) == 0 {
-		ctx.Error(http.StatusNotFound, err.Error())
+		ctx.Error(http.StatusNotFound)
 		return nil, nil
 	}
 

tests/integration/api_repo_test.go

@@ -684,7 +684,9 @@ func TestAPIRepoGetReviewers(t *testing.T) {
 	resp := MakeRequest(t, req, http.StatusOK)
 	var reviewers []*api.User
 	DecodeJSON(t, resp, &reviewers)
-	assert.Len(t, reviewers, 4)
+	if assert.Len(t, reviewers, 3) {
+		assert.ElementsMatch(t, []int64{1, 4, 11}, []int64{reviewers[0].ID, reviewers[1].ID, reviewers[2].ID})
+	}
 }
 
 func TestAPIRepoGetAssignees(t *testing.T) {