Merge remote-tracking branch 'giteaofficial/main'

* giteaofficial/main:
  Refactor some tests (go-gitea#34580)
  Do not mutate incoming options to SearchRepositoryByName (go-gitea#34553)
  Fix/improve avatar sync from LDAP (go-gitea#34573)
  Fix some trivial problems (go-gitea#34579)
  Retain issue sort type when a keyword search is introduced (go-gitea#34559)
  Always use an empty line to separate the commit message and trailer (go-gitea#34512)
  Fix line-button issue after file selection in file tree (go-gitea#34574)
  [skip ci] Updated translations via Crowdin
  Fix doctor deleting orphaned issues attachments (go-gitea#34142)
  [skip ci] Updated translations via Crowdin
  Fix actions skipped commit status indicator (go-gitea#34507)
  Clean up "file-view" related styles (go-gitea#34558)
  Add "View workflow file" to Actions list page (go-gitea#34538)
  Do not mutate incoming options to RenderUserSearch and SearchUsers  (go-gitea#34544)
  Add webhook assigning test and fix possible bug (go-gitea#34420)
  Fix possible nil description of pull request when migrating from CodeCommit (go-gitea#34541)
  Refactor commit reader (go-gitea#34542)

Author: zjjhot

cmd/admin.go

@@ -107,7 +107,7 @@ func runRepoSyncReleases(_ *cli.Context) error {
 
 	log.Trace("Synchronizing repository releases (this may take a while)")
 	for page := 1; ; page++ {
-		repos, count, err := repo_model.SearchRepositoryByName(ctx, &repo_model.SearchRepoOptions{
+		repos, count, err := repo_model.SearchRepositoryByName(ctx, repo_model.SearchRepoOptions{
 			ListOptions: db.ListOptions{
 				PageSize: repo_model.RepositoryListDefaultPageSize,
 				Page:     page,

cmd/admin_auth_ldap_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"code.gitea.io/gitea/models/auth"
+	"code.gitea.io/gitea/modules/test"
 	"code.gitea.io/gitea/services/auth/source/ldap"
 
 	"github.com/stretchr/testify/assert"
@@ -16,9 +17,7 @@ import (
 
 func TestAddLdapBindDn(t *testing.T) {
 	// Mock cli functions to do not exit on error
-	osExiter := cli.OsExiter
-	defer func() { cli.OsExiter = osExiter }()
-	cli.OsExiter = func(code int) {}
+	defer test.MockVariableValue(&cli.OsExiter, func(code int) {})()
 
 	// Test cases
 	cases := []struct {
@@ -256,9 +255,7 @@ func TestAddLdapBindDn(t *testing.T) {
 
 func TestAddLdapSimpleAuth(t *testing.T) {
 	// Mock cli functions to do not exit on error
-	osExiter := cli.OsExiter
-	defer func() { cli.OsExiter = osExiter }()
-	cli.OsExiter = func(code int) {}
+	defer test.MockVariableValue(&cli.OsExiter, func(code int) {})()
 
 	// Test cases
 	cases := []struct {
@@ -487,9 +484,7 @@ func TestAddLdapSimpleAuth(t *testing.T) {
 
 func TestUpdateLdapBindDn(t *testing.T) {
 	// Mock cli functions to do not exit on error
-	osExiter := cli.OsExiter
-	defer func() { cli.OsExiter = osExiter }()
-	cli.OsExiter = func(code int) {}
+	defer test.MockVariableValue(&cli.OsExiter, func(code int) {})()
 
 	// Test cases
 	cases := []struct {
@@ -964,9 +959,7 @@ func TestUpdateLdapBindDn(t *testing.T) {
 
 func TestUpdateLdapSimpleAuth(t *testing.T) {
 	// Mock cli functions to do not exit on error
-	osExiter := cli.OsExiter
-	defer func() { cli.OsExiter = osExiter }()
-	cli.OsExiter = func(code int) {}
+	defer test.MockVariableValue(&cli.OsExiter, func(code int) {})()
 
 	// Test cases
 	cases := []struct {

cmd/web_graceful.go

@@ -23,12 +23,6 @@ func NoHTTPRedirector() {
 	graceful.GetManager().InformCleanup()
 }
 
-// NoMainListener tells our cleanup routine that we will not be using a possibly provided listener
-// for our main HTTP/HTTPS service
-func NoMainListener() {
-	graceful.GetManager().InformCleanup()
-}
-
 // NoInstallListener tells our cleanup routine that we will not be using a possibly provided listener
 // for our install HTTP/HTTPS service
 func NoInstallListener() {

models/activities/user_heatmap.go

@@ -66,7 +66,7 @@ func getUserHeatmapData(ctx context.Context, user *user_model.User, team *organi
 		Select(groupBy+" AS timestamp, count(user_id) as contributions").
 		Table("action").
 		Where(cond).
-		And("created_unix > ?", timeutil.TimeStampNow()-31536000).
+		And("created_unix > ?", timeutil.TimeStampNow()-(366+7)*86400). // (366+7) days to include the first week for the heatmap
 		GroupBy(groupByName).
 		OrderBy("timestamp").
 		Find(&hdata)

models/fixtures/hook_task.yml

@@ -18,7 +18,7 @@
   id: 2
   hook_id: 1
   uuid: uuid2
-  is_delivered: false
+  is_delivered: true
 
 -
   id: 3

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})

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{}
 		}
 	}

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() {

models/issues/issue_update.go

@@ -12,9 +12,7 @@ import (
 	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/models/organization"
 	access_model "code.gitea.io/gitea/models/perm/access"
-	project_model "code.gitea.io/gitea/models/project"
 	repo_model "code.gitea.io/gitea/models/repo"
-	system_model "code.gitea.io/gitea/models/system"
 	"code.gitea.io/gitea/models/unit"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/git"
@@ -715,138 +713,13 @@ func UpdateReactionsMigrationsByType(ctx context.Context, gitServiceType api.Git
 	return err
 }
 
-// DeleteIssuesByRepoID deletes issues by repositories id
-func DeleteIssuesByRepoID(ctx context.Context, repoID int64) (attachmentPaths []string, err error) {
-	// MariaDB has a performance bug: https://jira.mariadb.org/browse/MDEV-16289
-	// so here it uses "DELETE ... WHERE IN" with pre-queried IDs.
-	sess := db.GetEngine(ctx)
-
-	for {
-		issueIDs := make([]int64, 0, db.DefaultMaxInSize)
-
-		err := sess.Table(&Issue{}).Where("repo_id = ?", repoID).OrderBy("id").Limit(db.DefaultMaxInSize).Cols("id").Find(&issueIDs)
-		if err != nil {
-			return nil, err
-		}
-
-		if len(issueIDs) == 0 {
-			break
-		}
-
-		// Delete content histories
-		_, err = sess.In("issue_id", issueIDs).Delete(&ContentHistory{})
-		if err != nil {
-			return nil, err
-		}
-
-		// Delete comments and attachments
-		_, err = sess.In("issue_id", issueIDs).Delete(&Comment{})
-		if err != nil {
-			return nil, err
-		}
-
-		// Dependencies for issues in this repository
-		_, err = sess.In("issue_id", issueIDs).Delete(&IssueDependency{})
-		if err != nil {
-			return nil, err
-		}
-
-		// Delete dependencies for issues in other repositories
-		_, err = sess.In("dependency_id", issueIDs).Delete(&IssueDependency{})
-		if err != nil {
-			return nil, err
-		}
-
-		_, err = sess.In("issue_id", issueIDs).Delete(&IssueUser{})
-		if err != nil {
-			return nil, err
-		}
-
-		_, err = sess.In("issue_id", issueIDs).Delete(&Reaction{})
-		if err != nil {
-			return nil, err
-		}
-
-		_, err = sess.In("issue_id", issueIDs).Delete(&IssueWatch{})
-		if err != nil {
-			return nil, err
-		}
-
-		_, err = sess.In("issue_id", issueIDs).Delete(&Stopwatch{})
-		if err != nil {
-			return nil, err
-		}
-
-		_, err = sess.In("issue_id", issueIDs).Delete(&TrackedTime{})
-		if err != nil {
-			return nil, err
-		}
-
-		_, err = sess.In("issue_id", issueIDs).Delete(&project_model.ProjectIssue{})
-		if err != nil {
-			return nil, err
-		}
-
-		_, err = sess.In("dependent_issue_id", issueIDs).Delete(&Comment{})
-		if err != nil {
-			return nil, err
-		}
-
-		var attachments []*repo_model.Attachment
-		err = sess.In("issue_id", issueIDs).Find(&attachments)
-		if err != nil {
-			return nil, err
-		}
-
-		for j := range attachments {
-			attachmentPaths = append(attachmentPaths, attachments[j].RelativePath())
-		}
-
-		_, err = sess.In("issue_id", issueIDs).Delete(&repo_model.Attachment{})
-		if err != nil {
-			return nil, err
-		}
-
-		_, err = sess.In("id", issueIDs).Delete(&Issue{})
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	return attachmentPaths, err
-}
-
-// DeleteOrphanedIssues delete issues without a repo
-func DeleteOrphanedIssues(ctx context.Context) error {
-	var attachmentPaths []string
-	err := db.WithTx(ctx, func(ctx context.Context) error {
-		var ids []int64
-
-		if err := db.GetEngine(ctx).Table("issue").Distinct("issue.repo_id").
-			Join("LEFT", "repository", "issue.repo_id=repository.id").
-			Where(builder.IsNull{"repository.id"}).GroupBy("issue.repo_id").
-			Find(&ids); err != nil {
-			return err
-		}
-
-		for i := range ids {
-			paths, err := DeleteIssuesByRepoID(ctx, ids[i])
-			if err != nil {
-				return err
-			}
-			attachmentPaths = append(attachmentPaths, paths...)
-		}
-
-		return nil
-	})
-	if err != nil {
-		return err
-	}
-
-	// Remove issue attachment files.
-	for i := range attachmentPaths {
-		// FIXME: it's not right, because the attachment might not be on local filesystem
-		system_model.RemoveAllWithNotice(ctx, "Delete issue attachment", attachmentPaths[i])
+func GetOrphanedIssueRepoIDs(ctx context.Context) ([]int64, error) {
+	var repoIDs []int64
+	if err := db.GetEngine(ctx).Table("issue").Distinct("issue.repo_id").
+		Join("LEFT", "repository", "issue.repo_id=repository.id").
+		Where(builder.IsNull{"repository.id"}).
+		Find(&repoIDs); err != nil {
+		return nil, err
 	}
-	return nil
+	return repoIDs, nil
 }

models/repo/pushmirror_test.go

@@ -39,8 +39,6 @@ func TestPushMirrorsIterate(t *testing.T) {
 		Interval:       0,
 	})
 
-	time.Sleep(1 * time.Millisecond)
-
 	repo_model.PushMirrorsIterate(db.DefaultContext, 1, func(idx int, bean any) error {
 		m, ok := bean.(*repo_model.PushMirror)
 		assert.True(t, ok)