From a789a8cc7a373990fbee33f25a91b537e275557f Mon Sep 17 00:00:00 2001 From: JIUN-TAI NIEN <44364165+nienjiuntai@users.noreply.github.com> Date: Tue, 24 Jun 2025 16:35:03 +0800 Subject: [PATCH 01/62] Fix job status aggregation logic (#34823) --- models/actions/run_job.go | 4 ++-- models/actions/run_job_status_test.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/models/actions/run_job.go b/models/actions/run_job.go index c0df19b020..bad895036d 100644 --- a/models/actions/run_job.go +++ b/models/actions/run_job.go @@ -185,10 +185,10 @@ func AggregateJobStatus(jobs []*ActionRunJob) Status { return StatusSuccess case hasCancelled: return StatusCancelled - case hasFailure: - return StatusFailure case hasRunning: return StatusRunning + case hasFailure: + return StatusFailure case hasWaiting: return StatusWaiting case hasBlocked: diff --git a/models/actions/run_job_status_test.go b/models/actions/run_job_status_test.go index 523d38327e..2a5eb00a6f 100644 --- a/models/actions/run_job_status_test.go +++ b/models/actions/run_job_status_test.go @@ -58,14 +58,14 @@ func TestAggregateJobStatus(t *testing.T) { {[]Status{StatusCancelled, StatusRunning}, StatusCancelled}, {[]Status{StatusCancelled, StatusBlocked}, StatusCancelled}, - // failure with other status, fail fast - // Should "running" win? Maybe no: old code does make "running" win, but GitHub does fail fast. + // failure with other status, usually fail fast, but "running" wins to match GitHub's behavior + // another reason that we can't make "failure" wins over "running": it would cause a weird behavior that user cannot cancel a workflow or get current running workflows correctly by filter after a job fail. {[]Status{StatusFailure}, StatusFailure}, {[]Status{StatusFailure, StatusSuccess}, StatusFailure}, {[]Status{StatusFailure, StatusSkipped}, StatusFailure}, {[]Status{StatusFailure, StatusCancelled}, StatusCancelled}, {[]Status{StatusFailure, StatusWaiting}, StatusFailure}, - {[]Status{StatusFailure, StatusRunning}, StatusFailure}, + {[]Status{StatusFailure, StatusRunning}, StatusRunning}, {[]Status{StatusFailure, StatusBlocked}, StatusFailure}, // skipped with other status From 6a97ab0af4031dd1e8fb0b272218e146b5556ac6 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 24 Jun 2025 21:24:09 +0800 Subject: [PATCH 02/62] Fix team permissions (#34827) * Fix #34793 * Fix #33456 --- models/git/protected_branch.go | 2 +- models/organization/org.go | 5 --- models/organization/team_repo.go | 33 +++++++------- models/organization/team_repo_test.go | 2 +- models/perm/access/repo_permission.go | 38 ++++++++-------- models/perm/access/repo_permission_test.go | 46 ++++++++++++++++++++ options/locale/locale_en-US.ini | 1 + routers/web/org/teams.go | 15 ++++++- routers/web/repo/setting/protected_branch.go | 3 +- routers/web/repo/setting/protected_tag.go | 3 +- services/convert/convert.go | 4 +- services/issue/assignee.go | 2 +- services/pull/reviewer.go | 2 +- templates/repo/settings/collaboration.tmpl | 26 +++++++++-- tests/integration/org_test.go | 33 ++++++++++++++ 15 files changed, 163 insertions(+), 52 deletions(-) diff --git a/models/git/protected_branch.go b/models/git/protected_branch.go index 19b02ccab9..55bbe6938c 100644 --- a/models/git/protected_branch.go +++ b/models/git/protected_branch.go @@ -518,7 +518,7 @@ func updateTeamWhitelist(ctx context.Context, repo *repo_model.Repository, curre return currentWhitelist, nil } - teams, err := organization.GetTeamsWithAccessToRepo(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead) + teams, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests) if err != nil { return nil, fmt.Errorf("GetTeamsWithAccessToRepo [org_id: %d, repo_id: %d]: %v", repo.OwnerID, repo.ID, err) } diff --git a/models/organization/org.go b/models/organization/org.go index dc889ea17f..0f3aef146c 100644 --- a/models/organization/org.go +++ b/models/organization/org.go @@ -602,8 +602,3 @@ func getUserTeamIDsQueryBuilder(orgID, userID int64) *builder.Builder { "team_user.uid": userID, }) } - -// TeamsWithAccessToRepo returns all teams that have given access level to the repository. -func (org *Organization) TeamsWithAccessToRepo(ctx context.Context, repoID int64, mode perm.AccessMode) ([]*Team, error) { - return GetTeamsWithAccessToRepo(ctx, org.ID, repoID, mode) -} diff --git a/models/organization/team_repo.go b/models/organization/team_repo.go index 53edd203a8..b3e266dbc7 100644 --- a/models/organization/team_repo.go +++ b/models/organization/team_repo.go @@ -9,6 +9,8 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/perm" "code.gitea.io/gitea/models/unit" + + "xorm.io/builder" ) // TeamRepo represents an team-repository relation. @@ -48,26 +50,27 @@ func RemoveTeamRepo(ctx context.Context, teamID, repoID int64) error { return err } -// GetTeamsWithAccessToRepo returns all teams in an organization that have given access level to the repository. -func GetTeamsWithAccessToRepo(ctx context.Context, orgID, repoID int64, mode perm.AccessMode) ([]*Team, error) { +// GetTeamsWithAccessToAnyRepoUnit returns all teams in an organization that have given access level to the repository special unit. +// This function is only used for finding some teams that can be used as branch protection allowlist or reviewers, it isn't really used for access control. +// FIXME: TEAM-UNIT-PERMISSION this logic is not complete, search the fixme keyword to see more details +func GetTeamsWithAccessToAnyRepoUnit(ctx context.Context, orgID, repoID int64, mode perm.AccessMode, unitType unit.Type, unitTypesMore ...unit.Type) ([]*Team, error) { teams := make([]*Team, 0, 5) - return teams, db.GetEngine(ctx).Where("team.authorize >= ?", mode). - Join("INNER", "team_repo", "team_repo.team_id = team.id"). - And("team_repo.org_id = ?", orgID). - And("team_repo.repo_id = ?", repoID). - OrderBy("name"). - Find(&teams) -} -// GetTeamsWithAccessToRepoUnit returns all teams in an organization that have given access level to the repository special unit. -func GetTeamsWithAccessToRepoUnit(ctx context.Context, orgID, repoID int64, mode perm.AccessMode, unitType unit.Type) ([]*Team, error) { - teams := make([]*Team, 0, 5) - return teams, db.GetEngine(ctx).Where("team_unit.access_mode >= ?", mode). + sub := builder.Select("team_id").From("team_unit"). + Where(builder.Expr("team_unit.team_id = team.id")). + And(builder.In("team_unit.type", append([]unit.Type{unitType}, unitTypesMore...))). + And(builder.Expr("team_unit.access_mode >= ?", mode)) + + err := db.GetEngine(ctx). Join("INNER", "team_repo", "team_repo.team_id = team.id"). - Join("INNER", "team_unit", "team_unit.team_id = team.id"). And("team_repo.org_id = ?", orgID). And("team_repo.repo_id = ?", repoID). - And("team_unit.type = ?", unitType). + And(builder.Or( + builder.Expr("team.authorize >= ?", mode), + builder.In("team.id", sub), + )). OrderBy("name"). Find(&teams) + + return teams, err } diff --git a/models/organization/team_repo_test.go b/models/organization/team_repo_test.go index c0d6750df9..73a06a93c2 100644 --- a/models/organization/team_repo_test.go +++ b/models/organization/team_repo_test.go @@ -22,7 +22,7 @@ func TestGetTeamsWithAccessToRepoUnit(t *testing.T) { org41 := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 41}) repo61 := unittest.AssertExistsAndLoadBean(t, &repo.Repository{ID: 61}) - teams, err := organization.GetTeamsWithAccessToRepoUnit(db.DefaultContext, org41.ID, repo61.ID, perm.AccessModeRead, unit.TypePullRequests) + teams, err := organization.GetTeamsWithAccessToAnyRepoUnit(db.DefaultContext, org41.ID, repo61.ID, perm.AccessModeRead, unit.TypePullRequests) assert.NoError(t, err) if assert.Len(t, teams, 2) { assert.EqualValues(t, 21, teams[0].ID) diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index 45efb192c8..fe429b1f97 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -267,7 +267,6 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use perm.units = repo.Units // anonymous user visit private repo. - // TODO: anonymous user visit public unit of private repo??? if user == nil && repo.IsPrivate { perm.AccessMode = perm_model.AccessModeNone return perm, nil @@ -286,7 +285,8 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use } // Prevent strangers from checking out public repo of private organization/users - // Allow user if they are collaborator of a repo within a private user or a private organization but not a member of the organization itself + // Allow user if they are a collaborator of a repo within a private user or a private organization but not a member of the organization itself + // TODO: rename it to "IsOwnerVisibleToDoer" if !organization.HasOrgOrUserVisible(ctx, repo.Owner, user) && !isCollaborator { perm.AccessMode = perm_model.AccessModeNone return perm, nil @@ -304,7 +304,7 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use return perm, nil } - // plain user + // plain user TODO: this check should be replaced, only need to check collaborator access mode perm.AccessMode, err = accessLevel(ctx, user, repo) if err != nil { return perm, err @@ -314,6 +314,19 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use return perm, nil } + // now: the owner is visible to doer, if the repo is public, then the min access mode is read + minAccessMode := util.Iif(!repo.IsPrivate && !user.IsRestricted, perm_model.AccessModeRead, perm_model.AccessModeNone) + perm.AccessMode = max(perm.AccessMode, minAccessMode) + + // get units mode from teams + teams, err := organization.GetUserRepoTeams(ctx, repo.OwnerID, user.ID, repo.ID) + if err != nil { + return perm, err + } + if len(teams) == 0 { + return perm, nil + } + perm.unitsMode = make(map[unit.Type]perm_model.AccessMode) // Collaborators on organization @@ -323,12 +336,6 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use } } - // get units mode from teams - teams, err := organization.GetUserRepoTeams(ctx, repo.OwnerID, user.ID, repo.ID) - if err != nil { - return perm, err - } - // if user in an owner team for _, team := range teams { if team.HasAdminAccess() { @@ -339,19 +346,12 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use } for _, u := range repo.Units { - var found bool for _, team := range teams { + unitAccessMode := minAccessMode if teamMode, exist := team.UnitAccessModeEx(ctx, u.Type); exist { - perm.unitsMode[u.Type] = max(perm.unitsMode[u.Type], teamMode) - found = true - } - } - - // for a public repo on an organization, a non-restricted user has read permission on non-team defined units. - if !found && !repo.IsPrivate && !user.IsRestricted { - if _, ok := perm.unitsMode[u.Type]; !ok { - perm.unitsMode[u.Type] = perm_model.AccessModeRead + unitAccessMode = max(perm.unitsMode[u.Type], unitAccessMode, teamMode) } + perm.unitsMode[u.Type] = unitAccessMode } } diff --git a/models/perm/access/repo_permission_test.go b/models/perm/access/repo_permission_test.go index 024f4400b3..c8675b1ded 100644 --- a/models/perm/access/repo_permission_test.go +++ b/models/perm/access/repo_permission_test.go @@ -6,12 +6,16 @@ package access import ( "testing" + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/organization" perm_model "code.gitea.io/gitea/models/perm" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" + "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestHasAnyUnitAccess(t *testing.T) { @@ -152,3 +156,45 @@ func TestUnitAccessMode(t *testing.T) { } assert.Equal(t, perm_model.AccessModeRead, perm.UnitAccessMode(unit.TypeWiki), "has unit, and map, use map") } + +func TestGetUserRepoPermission(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + ctx := t.Context() + repo32 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 32}) // org public repo + require.NoError(t, repo32.LoadOwner(ctx)) + require.True(t, repo32.Owner.IsOrganization()) + + require.NoError(t, db.TruncateBeans(ctx, &organization.Team{}, &organization.TeamUser{}, &organization.TeamRepo{}, &organization.TeamUnit{})) + org := repo32.Owner + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) + team := &organization.Team{OrgID: org.ID, LowerName: "test_team"} + require.NoError(t, db.Insert(ctx, team)) + + t.Run("DoerInTeamWithNoRepo", func(t *testing.T) { + require.NoError(t, db.Insert(ctx, &organization.TeamUser{OrgID: org.ID, TeamID: team.ID, UID: user.ID})) + perm, err := GetUserRepoPermission(ctx, repo32, user) + require.NoError(t, err) + assert.Equal(t, perm_model.AccessModeRead, perm.AccessMode) + assert.Nil(t, perm.unitsMode) // doer in the team, but has no access to the repo + }) + + require.NoError(t, db.Insert(ctx, &organization.TeamRepo{OrgID: org.ID, TeamID: team.ID, RepoID: repo32.ID})) + require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: org.ID, TeamID: team.ID, Type: unit.TypeCode, AccessMode: perm_model.AccessModeNone})) + t.Run("DoerWithTeamUnitAccessNone", func(t *testing.T) { + perm, err := GetUserRepoPermission(ctx, repo32, user) + require.NoError(t, err) + assert.Equal(t, perm_model.AccessModeRead, perm.AccessMode) + assert.Equal(t, perm_model.AccessModeRead, perm.unitsMode[unit.TypeCode]) + assert.Equal(t, perm_model.AccessModeRead, perm.unitsMode[unit.TypeIssues]) + }) + + require.NoError(t, db.TruncateBeans(ctx, &organization.TeamUnit{})) + require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: org.ID, TeamID: team.ID, Type: unit.TypeCode, AccessMode: perm_model.AccessModeWrite})) + t.Run("DoerWithTeamUnitAccessWrite", func(t *testing.T) { + perm, err := GetUserRepoPermission(ctx, repo32, user) + require.NoError(t, err) + assert.Equal(t, perm_model.AccessModeRead, perm.AccessMode) + assert.Equal(t, perm_model.AccessModeWrite, perm.unitsMode[unit.TypeCode]) + assert.Equal(t, perm_model.AccessModeRead, perm.unitsMode[unit.TypeIssues]) + }) +} diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 0efc9d04c2..3d3bafbffd 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2163,6 +2163,7 @@ settings.collaboration.write = Write settings.collaboration.read = Read settings.collaboration.owner = Owner settings.collaboration.undefined = Undefined +settings.collaboration.per_unit = Unit Permissions settings.hooks = Webhooks settings.githooks = Git Hooks settings.basic_settings = Basic Settings diff --git a/routers/web/org/teams.go b/routers/web/org/teams.go index 676c6d0c63..0ec7cfddc5 100644 --- a/routers/web/org/teams.go +++ b/routers/web/org/teams.go @@ -283,11 +283,22 @@ func NewTeam(ctx *context.Context) { } // FIXME: TEAM-UNIT-PERMISSION: this design is not right, when a new unit is added in the future, -// admin team won't inherit the correct admin permission for the new unit. +// The existing teams won't inherit the correct admin permission for the new unit. +// The full history is like this: +// 1. There was only "team", no "team unit", so "team.authorize" was used to determine the team permission. +// 2. Later, "team unit" was introduced, then the usage of "team.authorize" became inconsistent, and causes various bugs. +// - Sometimes, "team.authorize" is used to determine the team permission, e.g. admin, owner +// - Sometimes, "team unit" is used not really used and "team unit" is used. +// - Some functions like `GetTeamsWithAccessToAnyRepoUnit` use both. +// +// 3. After introducing "team unit" and more unclear changes, it becomes difficult to maintain team permissions. +// - Org owner need to click the permission for each unit, but can't just set a common "write" permission for all units. +// +// Ideally, "team.authorize=write" should mean the team has write access to all units including newly (future) added ones. func getUnitPerms(forms url.Values, teamPermission perm.AccessMode) map[unit_model.Type]perm.AccessMode { unitPerms := make(map[unit_model.Type]perm.AccessMode) for _, ut := range unit_model.AllRepoUnitTypes { - // Default accessmode is none + // Default access mode is none unitPerms[ut] = perm.AccessModeNone v, ok := forms[fmt.Sprintf("unit_%d", ut)] diff --git a/routers/web/repo/setting/protected_branch.go b/routers/web/repo/setting/protected_branch.go index f241242f02..0eea5e3f34 100644 --- a/routers/web/repo/setting/protected_branch.go +++ b/routers/web/repo/setting/protected_branch.go @@ -17,6 +17,7 @@ import ( "code.gitea.io/gitea/models/perm" access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/modules/web" @@ -89,7 +90,7 @@ func SettingsProtectedBranch(c *context.Context) { c.Data["recent_status_checks"] = contexts if c.Repo.Owner.IsOrganization() { - teams, err := organization.OrgFromUser(c.Repo.Owner).TeamsWithAccessToRepo(c, c.Repo.Repository.ID, perm.AccessModeRead) + teams, err := organization.GetTeamsWithAccessToAnyRepoUnit(c, c.Repo.Owner.ID, c.Repo.Repository.ID, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests) if err != nil { c.ServerError("Repo.Owner.TeamsWithAccessToRepo", err) return diff --git a/routers/web/repo/setting/protected_tag.go b/routers/web/repo/setting/protected_tag.go index 33692778d5..50f5a28c4c 100644 --- a/routers/web/repo/setting/protected_tag.go +++ b/routers/web/repo/setting/protected_tag.go @@ -12,6 +12,7 @@ import ( "code.gitea.io/gitea/models/organization" "code.gitea.io/gitea/models/perm" access_model "code.gitea.io/gitea/models/perm/access" + "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/templates" @@ -156,7 +157,7 @@ func setTagsContext(ctx *context.Context) error { ctx.Data["Users"] = users if ctx.Repo.Owner.IsOrganization() { - teams, err := organization.OrgFromUser(ctx.Repo.Owner).TeamsWithAccessToRepo(ctx, ctx.Repo.Repository.ID, perm.AccessModeRead) + teams, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, ctx.Repo.Owner.ID, ctx.Repo.Repository.ID, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests) if err != nil { ctx.ServerError("Repo.Owner.TeamsWithAccessToRepo", err) return err diff --git a/services/convert/convert.go b/services/convert/convert.go index 0723b0d34d..0de3822140 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -149,7 +149,7 @@ func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch, repo mergeWhitelistUsernames := getWhitelistEntities(readers, bp.MergeWhitelistUserIDs) approvalsWhitelistUsernames := getWhitelistEntities(readers, bp.ApprovalsWhitelistUserIDs) - teamReaders, err := organization.OrgFromUser(repo.Owner).TeamsWithAccessToRepo(ctx, repo.ID, perm.AccessModeRead) + teamReaders, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, repo.Owner.ID, repo.ID, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests) if err != nil { log.Error("Repo.Owner.TeamsWithAccessToRepo: %v", err) } @@ -727,7 +727,7 @@ func ToTagProtection(ctx context.Context, pt *git_model.ProtectedTag, repo *repo whitelistUsernames := getWhitelistEntities(readers, pt.AllowlistUserIDs) - teamReaders, err := organization.OrgFromUser(repo.Owner).TeamsWithAccessToRepo(ctx, repo.ID, perm.AccessModeRead) + teamReaders, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, repo.Owner.ID, repo.ID, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests) if err != nil { log.Error("Repo.Owner.TeamsWithAccessToRepo: %v", err) } diff --git a/services/issue/assignee.go b/services/issue/assignee.go index a9494b7ab4..ba9c91e0ed 100644 --- a/services/issue/assignee.go +++ b/services/issue/assignee.go @@ -304,7 +304,7 @@ func CanDoerChangeReviewRequests(ctx context.Context, doer *user_model.User, rep // If the repo's owner is an organization, members of teams with read permission on pull requests can change reviewers if repo.Owner.IsOrganization() { - teams, err := organization.GetTeamsWithAccessToRepo(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead) + teams, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead, unit.TypePullRequests) if err != nil { log.Error("GetTeamsWithAccessToRepo: %v", err) return false diff --git a/services/pull/reviewer.go b/services/pull/reviewer.go index bf0d8cb298..52f2f3401c 100644 --- a/services/pull/reviewer.go +++ b/services/pull/reviewer.go @@ -85,5 +85,5 @@ func GetReviewerTeams(ctx context.Context, repo *repo_model.Repository) ([]*orga return nil, nil } - return organization.GetTeamsWithAccessToRepoUnit(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead, unit.TypePullRequests) + return organization.GetTeamsWithAccessToAnyRepoUnit(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead, unit.TypePullRequests) } diff --git a/templates/repo/settings/collaboration.tmpl b/templates/repo/settings/collaboration.tmpl index 7064b4c7ba..e941a2c4b6 100644 --- a/templates/repo/settings/collaboration.tmpl +++ b/templates/repo/settings/collaboration.tmpl @@ -63,13 +63,33 @@ {{.Name}}
+ {{/*FIXME: TEAM-UNIT-PERMISSION this display is not right, search the fixme keyword to see more details */}} {{svg "octicon-shield-lock"}} - {{if eq .AccessMode 1}}{{ctx.Locale.Tr "repo.settings.collaboration.read"}}{{else if eq .AccessMode 2}}{{ctx.Locale.Tr "repo.settings.collaboration.write"}}{{else if eq .AccessMode 3}}{{ctx.Locale.Tr "repo.settings.collaboration.admin"}}{{else if eq .AccessMode 4}}{{ctx.Locale.Tr "repo.settings.collaboration.owner"}}{{else}}{{ctx.Locale.Tr "repo.settings.collaboration.undefined"}}{{end}} + {{if eq .AccessMode 0}} + {{ctx.Locale.Tr "repo.settings.collaboration.per_unit"}} + {{else if eq .AccessMode 1}} + {{ctx.Locale.Tr "repo.settings.collaboration.read"}} + {{else if eq .AccessMode 2}} + {{ctx.Locale.Tr "repo.settings.collaboration.write"}} + {{else if eq .AccessMode 3}} + {{ctx.Locale.Tr "repo.settings.collaboration.admin"}} + {{else if eq .AccessMode 4}} + {{ctx.Locale.Tr "repo.settings.collaboration.owner"}} + {{else}} + {{ctx.Locale.Tr "repo.settings.collaboration.undefined"}} + {{end}}
- {{if or (eq .AccessMode 1) (eq .AccessMode 2)}} + {{if or (eq .AccessMode 0) (eq .AccessMode 1) (eq .AccessMode 2)}} {{$first := true}}
- Sections: {{range $u, $unit := $.Units}}{{if and ($.Repo.UnitEnabled ctx $unit.Type) ($team.UnitEnabled ctx $unit.Type)}}{{if $first}}{{$first = false}}{{else}}, {{end}}{{ctx.Locale.Tr $unit.NameKey}}{{end}}{{end}} {{if $first}}None{{end}} + Units: + {{range $u, $unit := $.Units}} + {{- if and ($.Repo.UnitEnabled ctx $unit.Type) ($team.UnitEnabled ctx $unit.Type) -}} + {{- Iif $first "" ", "}}{{ctx.Locale.Tr $unit.NameKey -}} + {{- $first = false -}} + {{- end -}} + {{end}} + {{if $first}}None{{end}}
{{end}} diff --git a/tests/integration/org_test.go b/tests/integration/org_test.go index 0675648391..3ed7baa5ba 100644 --- a/tests/integration/org_test.go +++ b/tests/integration/org_test.go @@ -10,12 +10,17 @@ import ( "testing" auth_model "code.gitea.io/gitea/models/auth" + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/organization" + "code.gitea.io/gitea/models/perm" + "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestOrgRepos(t *testing.T) { @@ -217,4 +222,32 @@ func TestTeamSearch(t *testing.T) { session = loginUser(t, user5.Name) req = NewRequestf(t, "GET", "/org/%s/teams/-/search?q=%s", org.Name, "team") session.MakeRequest(t, req, http.StatusNotFound) + + t.Run("SearchWithPermission", func(t *testing.T) { + ctx := t.Context() + const testOrgID int64 = 500 + const testRepoID int64 = 2000 + testTeam := &organization.Team{OrgID: testOrgID, LowerName: "test_team", AccessMode: perm.AccessModeNone} + require.NoError(t, db.Insert(ctx, testTeam)) + require.NoError(t, db.Insert(ctx, &organization.TeamRepo{OrgID: testOrgID, TeamID: testTeam.ID, RepoID: testRepoID})) + require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: testOrgID, TeamID: testTeam.ID, Type: unit.TypeCode, AccessMode: perm.AccessModeRead})) + require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: testOrgID, TeamID: testTeam.ID, Type: unit.TypeIssues, AccessMode: perm.AccessModeWrite})) + + teams, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, testOrgID, testRepoID, perm.AccessModeRead, unit.TypeCode, unit.TypeIssues) + require.NoError(t, err) + assert.Len(t, teams, 1) // can read "code" or "issues" + + teams, err = organization.GetTeamsWithAccessToAnyRepoUnit(ctx, testOrgID, testRepoID, perm.AccessModeWrite, unit.TypeCode) + require.NoError(t, err) + assert.Empty(t, teams) // cannot write "code" + + teams, err = organization.GetTeamsWithAccessToAnyRepoUnit(ctx, testOrgID, testRepoID, perm.AccessModeWrite, unit.TypeIssues) + require.NoError(t, err) + assert.Len(t, teams, 1) // can write "issues" + + _, _ = db.GetEngine(ctx).ID(testTeam.ID).Update(&organization.Team{AccessMode: perm.AccessModeWrite}) + teams, err = organization.GetTeamsWithAccessToAnyRepoUnit(ctx, testOrgID, testRepoID, perm.AccessModeWrite, unit.TypeCode) + require.NoError(t, err) + assert.Len(t, teams, 1) // team permission is "write", so can write "code" + }) } From eb87e9d3b618718bd4a1a48cbb2b72e7bd6d02df Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 24 Jun 2025 21:51:04 +0800 Subject: [PATCH 03/62] Fix log fmt (#34810) --- services/lfs/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/lfs/server.go b/services/lfs/server.go index 59c9884fa8..15a51ad534 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -203,7 +203,7 @@ func BatchHandler(ctx *context.Context) { exists, err := contentStore.Exists(p) if err != nil { - log.Error("Unable to check if LFS OID[%s] exist. Error: %v", p.Oid, rc.User, rc.Repo, err) + log.Error("Unable to check if LFS object with ID '%s' exists for %s/%s. Error: %v", p.Oid, rc.User, rc.Repo, err) writeStatus(ctx, http.StatusInternalServerError) return } From 9a23fe131c567ff4f7e2b512a1bac4e3b98b1ba4 Mon Sep 17 00:00:00 2001 From: delvh Date: Tue, 24 Jun 2025 17:22:32 +0200 Subject: [PATCH 04/62] Ignore force pushes for changed files in a PR review (#34837) Fixes #34832 --------- Co-authored-by: wxiaoguang --- services/gitdiff/gitdiff.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 895cbe5a2f..010040bc70 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1356,6 +1356,7 @@ func SyncUserSpecificDiff(ctx context.Context, userID int64, pull *issues_model. // But as that does not work for all potential errors, we simply mark all files as unchanged and drop the error which always works, even if not as good as possible if err != nil { log.Error("Could not get changed files between %s and %s for pull request %d in repo with path %s. Assuming no changes. Error: %w", review.CommitSHA, latestCommit, pull.Index, gitRepo.Path, err) + err = nil //nolint } filesChangedSinceLastDiff := make(map[string]pull_model.ViewedState) @@ -1397,7 +1398,7 @@ outer: } } - return review, err + return review, nil } // CommentAsDiff returns c.Patch as *Diff From 22a84a72cd38d543ddb81b3dffabb5be25b8936d Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 24 Jun 2025 23:49:31 +0800 Subject: [PATCH 05/62] Fix SSH LFS timeout (#34838) Fix #34834 --- modules/lfstransfer/backend/util.go | 1 + services/context/private.go | 9 +++------ 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/modules/lfstransfer/backend/util.go b/modules/lfstransfer/backend/util.go index 98ce0b1e62..afe02f799c 100644 --- a/modules/lfstransfer/backend/util.go +++ b/modules/lfstransfer/backend/util.go @@ -132,6 +132,7 @@ func newInternalRequestLFS(ctx context.Context, internalURL, method string, head return nil } req := private.NewInternalRequest(ctx, internalURL, method) + req.SetReadWriteTimeout(0) for k, v := range headers { req.Header(k, v) } diff --git a/services/context/private.go b/services/context/private.go index 3f7637518b..d20e49f588 100644 --- a/services/context/private.go +++ b/services/context/private.go @@ -28,7 +28,6 @@ func init() { }) } -// Deadline is part of the interface for context.Context and we pass this to the request context func (ctx *PrivateContext) Deadline() (deadline time.Time, ok bool) { if ctx.Override != nil { return ctx.Override.Deadline() @@ -36,7 +35,6 @@ func (ctx *PrivateContext) Deadline() (deadline time.Time, ok bool) { return ctx.Base.Deadline() } -// Done is part of the interface for context.Context and we pass this to the request context func (ctx *PrivateContext) Done() <-chan struct{} { if ctx.Override != nil { return ctx.Override.Done() @@ -44,7 +42,6 @@ func (ctx *PrivateContext) Done() <-chan struct{} { return ctx.Base.Done() } -// Err is part of the interface for context.Context and we pass this to the request context func (ctx *PrivateContext) Err() error { if ctx.Override != nil { return ctx.Override.Err() @@ -52,14 +49,14 @@ func (ctx *PrivateContext) Err() error { return ctx.Base.Err() } -var privateContextKey any = "default_private_context" +type privateContextKeyType struct{} + +var privateContextKey privateContextKeyType -// GetPrivateContext returns a context for Private routes func GetPrivateContext(req *http.Request) *PrivateContext { return req.Context().Value(privateContextKey).(*PrivateContext) } -// PrivateContexter returns apicontext as middleware func PrivateContexter() func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { From 63fb25382bb84b1c0626fc206e585d509a3530ff Mon Sep 17 00:00:00 2001 From: silverwind Date: Tue, 24 Jun 2025 18:54:35 +0200 Subject: [PATCH 06/62] Remove unused variable HUGO_VERSION (#34840) This variable is unused, occurs nowhere in the codebase. I can't pinpoint the exact commit when it was last used, but I think it's unused since the docs were moved out. --- Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/Makefile b/Makefile index c868ef4463..230398ff24 100644 --- a/Makefile +++ b/Makefile @@ -81,7 +81,6 @@ ifeq ($(RACE_ENABLED),true) endif STORED_VERSION_FILE := VERSION -HUGO_VERSION ?= 0.111.3 GITHUB_REF_TYPE ?= branch GITHUB_REF_NAME ?= $(shell git rev-parse --abbrev-ref HEAD) From 0e629c545a4f922e2a708cc930b849b88ffc166a Mon Sep 17 00:00:00 2001 From: Junsik Kong Date: Wed, 25 Jun 2025 08:22:58 +0900 Subject: [PATCH 07/62] fix(issue): Replace stopwatch toggle with explicit start/stop actions (#34818) This PR fixes a state de-synchronization bug with the issue stopwatch, it resolves the issue by replacing the ambiguous `/toggle` endpoint with two explicit endpoints: `/start` and `/stop`. - The "Start timer" button now exclusively calls the `/start` endpoint. - The "Stop timer" button now exclusively calls the `/stop` endpoint. This ensures the user's intent is clearly communicated to the server, eliminating the state inconsistency and fixing the bug. --------- Signed-off-by: wxiaoguang Co-authored-by: wxiaoguang --- models/issues/stopwatch.go | 169 ++++++------------ models/issues/stopwatch_test.go | 61 ++++--- options/locale/locale_en-US.ini | 2 + routers/api/v1/repo/issue_stopwatch.go | 56 +++--- routers/web/repo/issue_stopwatch.go | 40 +++-- routers/web/repo/pull.go | 9 +- routers/web/web.go | 3 +- services/issue/status.go | 4 +- templates/base/head_navbar.tmpl | 2 +- .../issue/sidebar/stopwatch_timetracker.tmpl | 4 +- templates/swagger/v1_json.tmpl | 4 +- tests/integration/timetracking_test.go | 10 +- web_src/js/features/stopwatch.ts | 2 +- 13 files changed, 162 insertions(+), 204 deletions(-) diff --git a/models/issues/stopwatch.go b/models/issues/stopwatch.go index 7c05a3a883..761b8f91a0 100644 --- a/models/issues/stopwatch.go +++ b/models/issues/stopwatch.go @@ -5,7 +5,6 @@ package issues import ( "context" - "fmt" "time" "code.gitea.io/gitea/models/db" @@ -15,20 +14,6 @@ import ( "code.gitea.io/gitea/modules/util" ) -// ErrIssueStopwatchNotExist represents an error that stopwatch is not exist -type ErrIssueStopwatchNotExist struct { - UserID int64 - IssueID int64 -} - -func (err ErrIssueStopwatchNotExist) Error() string { - return fmt.Sprintf("issue stopwatch doesn't exist[uid: %d, issue_id: %d", err.UserID, err.IssueID) -} - -func (err ErrIssueStopwatchNotExist) Unwrap() error { - return util.ErrNotExist -} - // Stopwatch represents a stopwatch for time tracking. type Stopwatch struct { ID int64 `xorm:"pk autoincr"` @@ -55,13 +40,11 @@ func getStopwatch(ctx context.Context, userID, issueID int64) (sw *Stopwatch, ex return sw, exists, err } -// UserIDCount is a simple coalition of UserID and Count type UserStopwatch struct { UserID int64 StopWatches []*Stopwatch } -// GetUIDsAndNotificationCounts between the two provided times func GetUIDsAndStopwatch(ctx context.Context) ([]*UserStopwatch, error) { sws := []*Stopwatch{} if err := db.GetEngine(ctx).Where("issue_id != 0").Find(&sws); err != nil { @@ -87,7 +70,7 @@ func GetUIDsAndStopwatch(ctx context.Context) ([]*UserStopwatch, error) { return res, nil } -// GetUserStopwatches return list of all stopwatches of a user +// GetUserStopwatches return list of the user's all stopwatches func GetUserStopwatches(ctx context.Context, userID int64, listOptions db.ListOptions) ([]*Stopwatch, error) { sws := make([]*Stopwatch, 0, 8) sess := db.GetEngine(ctx).Where("stopwatch.user_id = ?", userID) @@ -102,7 +85,7 @@ func GetUserStopwatches(ctx context.Context, userID int64, listOptions db.ListOp return sws, nil } -// CountUserStopwatches return count of all stopwatches of a user +// CountUserStopwatches return count of the user's all stopwatches func CountUserStopwatches(ctx context.Context, userID int64) (int64, error) { return db.GetEngine(ctx).Where("user_id = ?", userID).Count(&Stopwatch{}) } @@ -136,43 +119,21 @@ func HasUserStopwatch(ctx context.Context, userID int64) (exists bool, sw *Stopw return exists, sw, issue, err } -// FinishIssueStopwatchIfPossible if stopwatch exist then finish it otherwise ignore -func FinishIssueStopwatchIfPossible(ctx context.Context, user *user_model.User, issue *Issue) error { - _, exists, err := getStopwatch(ctx, user.ID, issue.ID) - if err != nil { - return err - } - if !exists { - return nil - } - return FinishIssueStopwatch(ctx, user, issue) -} - -// CreateOrStopIssueStopwatch create an issue stopwatch if it's not exist, otherwise finish it -func CreateOrStopIssueStopwatch(ctx context.Context, user *user_model.User, issue *Issue) error { - _, exists, err := getStopwatch(ctx, user.ID, issue.ID) - if err != nil { - return err - } - if exists { - return FinishIssueStopwatch(ctx, user, issue) - } - return CreateIssueStopwatch(ctx, user, issue) -} - -// FinishIssueStopwatch if stopwatch exist then finish it otherwise return an error -func FinishIssueStopwatch(ctx context.Context, user *user_model.User, issue *Issue) error { +// FinishIssueStopwatch if stopwatch exists, then finish it. +func FinishIssueStopwatch(ctx context.Context, user *user_model.User, issue *Issue) (ok bool, err error) { sw, exists, err := getStopwatch(ctx, user.ID, issue.ID) if err != nil { - return err + return false, err + } else if !exists { + return false, nil } - if !exists { - return ErrIssueStopwatchNotExist{ - UserID: user.ID, - IssueID: issue.ID, - } + if err = finishIssueStopwatch(ctx, user, issue, sw); err != nil { + return false, err } + return true, nil +} +func finishIssueStopwatch(ctx context.Context, user *user_model.User, issue *Issue, sw *Stopwatch) error { // Create tracked time out of the time difference between start date and actual date timediff := time.Now().Unix() - int64(sw.CreatedUnix) @@ -184,14 +145,12 @@ func FinishIssueStopwatch(ctx context.Context, user *user_model.User, issue *Iss Time: timediff, } - if err := db.Insert(ctx, tt); err != nil { - return err - } - if err := issue.LoadRepo(ctx); err != nil { return err } - + if err := db.Insert(ctx, tt); err != nil { + return err + } if _, err := CreateComment(ctx, &CreateCommentOptions{ Doer: user, Issue: issue, @@ -202,83 +161,65 @@ func FinishIssueStopwatch(ctx context.Context, user *user_model.User, issue *Iss }); err != nil { return err } - _, err = db.DeleteByBean(ctx, sw) + _, err := db.DeleteByBean(ctx, sw) return err } -// CreateIssueStopwatch creates a stopwatch if not exist, otherwise return an error -func CreateIssueStopwatch(ctx context.Context, user *user_model.User, issue *Issue) error { - if err := issue.LoadRepo(ctx); err != nil { - return err - } - - // if another stopwatch is running: stop it - exists, _, otherIssue, err := HasUserStopwatch(ctx, user.ID) - if err != nil { - return err - } - if exists { - if err := FinishIssueStopwatch(ctx, user, otherIssue); err != nil { - return err +// CreateIssueStopwatch creates a stopwatch if the issue doesn't have the user's stopwatch. +// It also stops any other stopwatch that might be running for the user. +func CreateIssueStopwatch(ctx context.Context, user *user_model.User, issue *Issue) (ok bool, err error) { + { // if another issue's stopwatch is running: stop it; if this issue has a stopwatch: return an error. + exists, otherStopWatch, otherIssue, err := HasUserStopwatch(ctx, user.ID) + if err != nil { + return false, err + } + if exists { + if otherStopWatch.IssueID == issue.ID { + // don't allow starting stopwatch for the same issue + return false, nil + } + // stop the other issue's stopwatch + if err = finishIssueStopwatch(ctx, user, otherIssue, otherStopWatch); err != nil { + return false, err + } } } - // Create stopwatch - sw := &Stopwatch{ - UserID: user.ID, - IssueID: issue.ID, + if err = issue.LoadRepo(ctx); err != nil { + return false, err } - - if err := db.Insert(ctx, sw); err != nil { - return err + if err = db.Insert(ctx, &Stopwatch{UserID: user.ID, IssueID: issue.ID}); err != nil { + return false, err } - - if err := issue.LoadRepo(ctx); err != nil { - return err - } - - if _, err := CreateComment(ctx, &CreateCommentOptions{ + if _, err = CreateComment(ctx, &CreateCommentOptions{ Doer: user, Issue: issue, Repo: issue.Repo, Type: CommentTypeStartTracking, }); err != nil { - return err + return false, err } - - return nil + return true, nil } // CancelStopwatch removes the given stopwatch and logs it into issue's timeline. -func CancelStopwatch(ctx context.Context, user *user_model.User, issue *Issue) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - if err := cancelStopwatch(ctx, user, issue); err != nil { - return err - } - return committer.Commit() -} - -func cancelStopwatch(ctx context.Context, user *user_model.User, issue *Issue) error { - e := db.GetEngine(ctx) - sw, exists, err := getStopwatch(ctx, user.ID, issue.ID) - if err != nil { - return err - } - - if exists { - if _, err := e.Delete(sw); err != nil { +func CancelStopwatch(ctx context.Context, user *user_model.User, issue *Issue) (ok bool, err error) { + err = db.WithTx(ctx, func(ctx context.Context) error { + e := db.GetEngine(ctx) + sw, exists, err := getStopwatch(ctx, user.ID, issue.ID) + if err != nil { return err + } else if !exists { + return nil } - if err := issue.LoadRepo(ctx); err != nil { + if err = issue.LoadRepo(ctx); err != nil { return err } - - if _, err := CreateComment(ctx, &CreateCommentOptions{ + if _, err = e.Delete(sw); err != nil { + return err + } + if _, err = CreateComment(ctx, &CreateCommentOptions{ Doer: user, Issue: issue, Repo: issue.Repo, @@ -286,6 +227,8 @@ func cancelStopwatch(ctx context.Context, user *user_model.User, issue *Issue) e }); err != nil { return err } - } - return nil + ok = true + return nil + }) + return ok, err } diff --git a/models/issues/stopwatch_test.go b/models/issues/stopwatch_test.go index a1bf9dc931..6333c10234 100644 --- a/models/issues/stopwatch_test.go +++ b/models/issues/stopwatch_test.go @@ -10,7 +10,6 @@ import ( issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/timeutil" "github.com/stretchr/testify/assert" ) @@ -18,26 +17,22 @@ import ( func TestCancelStopwatch(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - user1, err := user_model.GetUserByID(db.DefaultContext, 1) - assert.NoError(t, err) + user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + issue1 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1}) - issue1, err := issues_model.GetIssueByID(db.DefaultContext, 1) - assert.NoError(t, err) - issue2, err := issues_model.GetIssueByID(db.DefaultContext, 2) - assert.NoError(t, err) - - err = issues_model.CancelStopwatch(db.DefaultContext, user1, issue1) + ok, err := issues_model.CancelStopwatch(db.DefaultContext, user1, issue1) assert.NoError(t, err) + assert.True(t, ok) unittest.AssertNotExistsBean(t, &issues_model.Stopwatch{UserID: user1.ID, IssueID: issue1.ID}) + unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{Type: issues_model.CommentTypeCancelTracking, PosterID: user1.ID, IssueID: issue1.ID}) - _ = unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{Type: issues_model.CommentTypeCancelTracking, PosterID: user1.ID, IssueID: issue1.ID}) - - assert.NoError(t, issues_model.CancelStopwatch(db.DefaultContext, user1, issue2)) + ok, err = issues_model.CancelStopwatch(db.DefaultContext, user1, issue1) + assert.NoError(t, err) + assert.False(t, ok) } func TestStopwatchExists(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - assert.True(t, issues_model.StopwatchExists(db.DefaultContext, 1, 1)) assert.False(t, issues_model.StopwatchExists(db.DefaultContext, 1, 2)) } @@ -58,21 +53,35 @@ func TestHasUserStopwatch(t *testing.T) { func TestCreateOrStopIssueStopwatch(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - user2, err := user_model.GetUserByID(db.DefaultContext, 2) - assert.NoError(t, err) - org3, err := user_model.GetUserByID(db.DefaultContext, 3) - assert.NoError(t, err) + user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) + issue1 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1}) + issue3 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 3}) - issue1, err := issues_model.GetIssueByID(db.DefaultContext, 1) + // create a new stopwatch + ok, err := issues_model.CreateIssueStopwatch(db.DefaultContext, user4, issue1) assert.NoError(t, err) - issue2, err := issues_model.GetIssueByID(db.DefaultContext, 2) + assert.True(t, ok) + unittest.AssertExistsAndLoadBean(t, &issues_model.Stopwatch{UserID: user4.ID, IssueID: issue1.ID}) + // should not create a second stopwatch for the same issue + ok, err = issues_model.CreateIssueStopwatch(db.DefaultContext, user4, issue1) assert.NoError(t, err) + assert.False(t, ok) + // on a different issue, it will finish the existing stopwatch and create a new one + ok, err = issues_model.CreateIssueStopwatch(db.DefaultContext, user4, issue3) + assert.NoError(t, err) + assert.True(t, ok) + unittest.AssertNotExistsBean(t, &issues_model.Stopwatch{UserID: user4.ID, IssueID: issue1.ID}) + unittest.AssertExistsAndLoadBean(t, &issues_model.Stopwatch{UserID: user4.ID, IssueID: issue3.ID}) - assert.NoError(t, issues_model.CreateOrStopIssueStopwatch(db.DefaultContext, org3, issue1)) - sw := unittest.AssertExistsAndLoadBean(t, &issues_model.Stopwatch{UserID: 3, IssueID: 1}) - assert.LessOrEqual(t, sw.CreatedUnix, timeutil.TimeStampNow()) - - assert.NoError(t, issues_model.CreateOrStopIssueStopwatch(db.DefaultContext, user2, issue2)) - unittest.AssertNotExistsBean(t, &issues_model.Stopwatch{UserID: 2, IssueID: 2}) - unittest.AssertExistsAndLoadBean(t, &issues_model.TrackedTime{UserID: 2, IssueID: 2}) + // user2 already has a stopwatch in test fixture + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + issue2 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2}) + ok, err = issues_model.FinishIssueStopwatch(db.DefaultContext, user2, issue2) + assert.NoError(t, err) + assert.True(t, ok) + unittest.AssertNotExistsBean(t, &issues_model.Stopwatch{UserID: user2.ID, IssueID: issue2.ID}) + unittest.AssertExistsAndLoadBean(t, &issues_model.TrackedTime{UserID: user2.ID, IssueID: issue2.ID}) + ok, err = issues_model.FinishIssueStopwatch(db.DefaultContext, user2, issue2) + assert.NoError(t, err) + assert.False(t, ok) } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 3d3bafbffd..b478d02593 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1726,6 +1726,8 @@ issues.remove_time_estimate_at = removed time estimate %s issues.time_estimate_invalid = Time estimate format is invalid issues.start_tracking_history = started working %s issues.tracker_auto_close = Timer will be stopped automatically when this issue gets closed +issues.stopwatch_already_stopped = The timer for this issue is already stopped +issues.stopwatch_already_created = The timer for this issue already exists issues.tracking_already_started = `You have already started time tracking on another issue!` issues.stop_tracking = Stop Timer issues.stop_tracking_history = worked for %[1]s %[2]s diff --git a/routers/api/v1/repo/issue_stopwatch.go b/routers/api/v1/repo/issue_stopwatch.go index b18e172b37..0f28b9757d 100644 --- a/routers/api/v1/repo/issue_stopwatch.go +++ b/routers/api/v1/repo/issue_stopwatch.go @@ -4,7 +4,6 @@ package repo import ( - "errors" "net/http" issues_model "code.gitea.io/gitea/models/issues" @@ -49,14 +48,17 @@ func StartIssueStopwatch(ctx *context.APIContext) { // "409": // description: Cannot start a stopwatch again if it already exists - issue, err := prepareIssueStopwatch(ctx, false) - if err != nil { + issue := prepareIssueForStopwatch(ctx) + if ctx.Written() { return } - if err := issues_model.CreateIssueStopwatch(ctx, ctx.Doer, issue); err != nil { + if ok, err := issues_model.CreateIssueStopwatch(ctx, ctx.Doer, issue); err != nil { ctx.APIErrorInternal(err) return + } else if !ok { + ctx.APIError(http.StatusConflict, "cannot start a stopwatch again if it already exists") + return } ctx.Status(http.StatusCreated) @@ -96,18 +98,20 @@ func StopIssueStopwatch(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" // "409": - // description: Cannot stop a non existent stopwatch + // description: Cannot stop a non-existent stopwatch - issue, err := prepareIssueStopwatch(ctx, true) - if err != nil { + issue := prepareIssueForStopwatch(ctx) + if ctx.Written() { return } - if err := issues_model.FinishIssueStopwatch(ctx, ctx.Doer, issue); err != nil { + if ok, err := issues_model.FinishIssueStopwatch(ctx, ctx.Doer, issue); err != nil { ctx.APIErrorInternal(err) return + } else if !ok { + ctx.APIError(http.StatusConflict, "cannot stop a non-existent stopwatch") + return } - ctx.Status(http.StatusCreated) } @@ -145,22 +149,25 @@ func DeleteIssueStopwatch(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" // "409": - // description: Cannot cancel a non existent stopwatch + // description: Cannot cancel a non-existent stopwatch - issue, err := prepareIssueStopwatch(ctx, true) - if err != nil { + issue := prepareIssueForStopwatch(ctx) + if ctx.Written() { return } - if err := issues_model.CancelStopwatch(ctx, ctx.Doer, issue); err != nil { + if ok, err := issues_model.CancelStopwatch(ctx, ctx.Doer, issue); err != nil { ctx.APIErrorInternal(err) return + } else if !ok { + ctx.APIError(http.StatusConflict, "cannot cancel a non-existent stopwatch") + return } ctx.Status(http.StatusNoContent) } -func prepareIssueStopwatch(ctx *context.APIContext, shouldExist bool) (*issues_model.Issue, error) { +func prepareIssueForStopwatch(ctx *context.APIContext) *issues_model.Issue { issue, err := issues_model.GetIssueByIndex(ctx, ctx.Repo.Repository.ID, ctx.PathParamInt64("index")) if err != nil { if issues_model.IsErrIssueNotExist(err) { @@ -168,32 +175,19 @@ func prepareIssueStopwatch(ctx *context.APIContext, shouldExist bool) (*issues_m } else { ctx.APIErrorInternal(err) } - - return nil, err + return nil } if !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) { ctx.Status(http.StatusForbidden) - return nil, errors.New("Unable to write to PRs") + return nil } if !ctx.Repo.CanUseTimetracker(ctx, issue, ctx.Doer) { ctx.Status(http.StatusForbidden) - return nil, errors.New("Cannot use time tracker") + return nil } - - if issues_model.StopwatchExists(ctx, ctx.Doer.ID, issue.ID) != shouldExist { - if shouldExist { - ctx.APIError(http.StatusConflict, "cannot stop/cancel a non existent stopwatch") - err = errors.New("cannot stop/cancel a non existent stopwatch") - } else { - ctx.APIError(http.StatusConflict, "cannot start a stopwatch again if it already exists") - err = errors.New("cannot start a stopwatch again if it already exists") - } - return nil, err - } - - return issue, nil + return issue } // GetStopwatches get all stopwatches diff --git a/routers/web/repo/issue_stopwatch.go b/routers/web/repo/issue_stopwatch.go index 5a8d203771..2de3a7cfec 100644 --- a/routers/web/repo/issue_stopwatch.go +++ b/routers/web/repo/issue_stopwatch.go @@ -10,33 +10,47 @@ import ( "code.gitea.io/gitea/services/context" ) -// IssueStopwatch creates or stops a stopwatch for the given issue. -func IssueStopwatch(c *context.Context) { +// IssueStartStopwatch creates a stopwatch for the given issue. +func IssueStartStopwatch(c *context.Context) { issue := GetActionIssue(c) if c.Written() { return } - var showSuccessMessage bool - - if !issues_model.StopwatchExists(c, c.Doer.ID, issue.ID) { - showSuccessMessage = true - } - if !c.Repo.CanUseTimetracker(c, issue, c.Doer) { c.NotFound(nil) return } - if err := issues_model.CreateOrStopIssueStopwatch(c, c.Doer, issue); err != nil { - c.ServerError("CreateOrStopIssueStopwatch", err) + if ok, err := issues_model.CreateIssueStopwatch(c, c.Doer, issue); err != nil { + c.ServerError("CreateIssueStopwatch", err) + return + } else if !ok { + c.Flash.Warning(c.Tr("repo.issues.stopwatch_already_created")) + } else { + c.Flash.Success(c.Tr("repo.issues.tracker_auto_close")) + } + c.JSONRedirect("") +} + +// IssueStopStopwatch stops a stopwatch for the given issue. +func IssueStopStopwatch(c *context.Context) { + issue := GetActionIssue(c) + if c.Written() { return } - if showSuccessMessage { - c.Flash.Success(c.Tr("repo.issues.tracker_auto_close")) + if !c.Repo.CanUseTimetracker(c, issue, c.Doer) { + c.NotFound(nil) + return } + if ok, err := issues_model.FinishIssueStopwatch(c, c.Doer, issue); err != nil { + c.ServerError("FinishIssueStopwatch", err) + return + } else if !ok { + c.Flash.Warning(c.Tr("repo.issues.stopwatch_already_stopped")) + } c.JSONRedirect("") } @@ -51,7 +65,7 @@ func CancelStopwatch(c *context.Context) { return } - if err := issues_model.CancelStopwatch(c, c.Doer, issue); err != nil { + if _, err := issues_model.CancelStopwatch(c, c.Doer, issue); err != nil { c.ServerError("CancelStopwatch", err) return } diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 9176b14cd4..f662152e2e 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1258,13 +1258,8 @@ func CancelAutoMergePullRequest(ctx *context.Context) { } func stopTimerIfAvailable(ctx *context.Context, user *user_model.User, issue *issues_model.Issue) error { - if issues_model.StopwatchExists(ctx, user.ID, issue.ID) { - if err := issues_model.CreateOrStopIssueStopwatch(ctx, user, issue); err != nil { - return err - } - } - - return nil + _, err := issues_model.FinishIssueStopwatch(ctx, user, issue) + return err } func PullsNewRedirect(ctx *context.Context) { diff --git a/routers/web/web.go b/routers/web/web.go index 4b5d68b260..66a5a9be5e 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1253,7 +1253,8 @@ func registerWebRoutes(m *web.Router) { m.Post("/add", web.Bind(forms.AddTimeManuallyForm{}), repo.AddTimeManually) m.Post("/{timeid}/delete", repo.DeleteTime) m.Group("/stopwatch", func() { - m.Post("/toggle", repo.IssueStopwatch) + m.Post("/start", repo.IssueStartStopwatch) + m.Post("/stop", repo.IssueStopStopwatch) m.Post("/cancel", repo.CancelStopwatch) }) }) diff --git a/services/issue/status.go b/services/issue/status.go index e18b891175..f9d7dca841 100644 --- a/services/issue/status.go +++ b/services/issue/status.go @@ -24,14 +24,14 @@ func CloseIssue(ctx context.Context, issue *issues_model.Issue, doer *user_model comment, err := issues_model.CloseIssue(dbCtx, issue, doer) if err != nil { if issues_model.IsErrDependenciesLeft(err) { - if err := issues_model.FinishIssueStopwatchIfPossible(dbCtx, doer, issue); err != nil { + if _, err := issues_model.FinishIssueStopwatch(dbCtx, doer, issue); err != nil { log.Error("Unable to stop stopwatch for issue[%d]#%d: %v", issue.ID, issue.Index, err) } } return err } - if err := issues_model.FinishIssueStopwatchIfPossible(dbCtx, doer, issue); err != nil { + if _, err := issues_model.FinishIssueStopwatch(dbCtx, doer, issue); err != nil { return err } diff --git a/templates/base/head_navbar.tmpl b/templates/base/head_navbar.tmpl index 35e14d38d3..dacc518873 100644 --- a/templates/base/head_navbar.tmpl +++ b/templates/base/head_navbar.tmpl @@ -197,7 +197,7 @@ {{$activeStopwatch.RepoSlug}}#{{$activeStopwatch.IssueIndex}}
-
+ {{.CsrfTokenHtml}}
- {{template "shared/repo_search" .}} + {{template "shared/repo/search" .}}
From aa9d86745ae1ebe29f284f783bb085e4deea50c9 Mon Sep 17 00:00:00 2001 From: TheFox0x7 Date: Fri, 27 Jun 2025 15:48:03 +0200 Subject: [PATCH 21/62] enforce explanation for necessary nolints and fix bugs (#34883) Follows up https://github.com/go-gitea/gitea/pull/34851 --------- Co-authored-by: wxiaoguang --- .golangci.yml | 3 +- cmd/embedded.go | 8 ++- contrib/backport/backport.go | 2 +- models/actions/task.go | 9 ++-- models/auth/auth_token.go | 2 +- models/db/sql_postgres_with_schema.go | 4 +- models/migrations/base/tests.go | 5 +- models/migrations/v1_11/v112.go | 4 +- models/migrations/v1_13/v140.go | 7 +-- models/migrations/v1_14/v157.go | 11 ---- models/migrations/v1_14/v165.go | 10 +--- models/user/badge.go | 2 +- modules/auth/password/hash/common.go | 2 +- modules/cache/cache_redis.go | 2 +- modules/cache/cache_twoqueue.go | 2 +- modules/cache/string_cache.go | 2 +- modules/graceful/manager_windows.go | 3 +- modules/json/json.go | 3 +- modules/log/logger.go | 2 +- .../markup/markdown/transform_blockquote.go | 2 +- modules/markup/markdown/transform_codespan.go | 2 +- modules/markup/markdown/transform_heading.go | 2 +- modules/markup/mdstripper/mdstripper.go | 2 +- modules/optional/serialization_test.go | 2 +- modules/setting/config_env.go | 2 +- modules/setting/config_env_test.go | 3 ++ modules/setting/config_provider.go | 2 +- modules/setting/security.go | 2 +- modules/setting/storage.go | 6 +-- modules/templates/htmlrenderer.go | 4 +- modules/templates/scopedtmpl/scopedtmpl.go | 25 ++++------ routers/api/actions/artifacts_utils.go | 2 +- routers/api/packages/container/blob.go | 2 +- routers/api/packages/nuget/nuget.go | 2 +- routers/api/v1/misc/markup.go | 4 +- routers/web/admin/config.go | 2 +- routers/web/misc/markup.go | 2 +- routers/web/web.go | 4 +- services/actions/notifier_helper.go | 7 +-- services/auth/source/ldap/source_search.go | 4 +- services/doctor/paths.go | 5 +- services/gitdiff/gitdiff.go | 50 +------------------ services/migrations/migrate.go | 8 ++- services/packages/rpm/repository.go | 4 +- services/pull/merge.go | 2 +- tests/e2e/e2e_test.go | 2 +- tests/integration/integration_test.go | 2 +- tests/test_utils.go | 3 +- 48 files changed, 83 insertions(+), 159 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 0fcb62b8d2..70efd288ff 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -46,7 +46,8 @@ linters: - pkg: gitea.com/go-chi/cache desc: do not use the go-chi cache package, use gitea's cache system nolintlint: - # require-explanation: true + allow-unused: false + require-explanation: true require-specific: true gocritic: disabled-checks: diff --git a/cmd/embedded.go b/cmd/embedded.go index 6a2fa07a93..1908352453 100644 --- a/cmd/embedded.go +++ b/cmd/embedded.go @@ -295,16 +295,14 @@ func collectAssetFilesByPattern(c *cli.Command, globs []glob.Glob, path string, } } -func compileCollectPatterns(args []string) ([]glob.Glob, error) { +func compileCollectPatterns(args []string) (_ []glob.Glob, err error) { if len(args) == 0 { args = []string{"**"} } pat := make([]glob.Glob, len(args)) for i := range args { - if g, err := glob.Compile(args[i], '/'); err != nil { - return nil, fmt.Errorf("'%s': Invalid glob pattern: %w", args[i], err) - } else { //nolint:revive - pat[i] = g + if pat[i], err = glob.Compile(args[i], '/'); err != nil { + return nil, fmt.Errorf("invalid glob patterh %q: %w", args[i], err) } } return pat, nil diff --git a/contrib/backport/backport.go b/contrib/backport/backport.go index 6fbd610e62..2052295fb1 100644 --- a/contrib/backport/backport.go +++ b/contrib/backport/backport.go @@ -1,7 +1,7 @@ // Copyright 2023 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -//nolint:forbidigo +//nolint:forbidigo // use of print functions is allowed in cli package main import ( diff --git a/models/actions/task.go b/models/actions/task.go index 63259582f6..e0756b10c2 100644 --- a/models/actions/task.go +++ b/models/actions/task.go @@ -278,14 +278,13 @@ func CreateTaskForRunner(ctx context.Context, runner *ActionRunner) (*ActionTask return nil, false, err } - var workflowJob *jobparser.Job - if gots, err := jobparser.Parse(job.WorkflowPayload); err != nil { + parsedWorkflows, err := jobparser.Parse(job.WorkflowPayload) + if err != nil { return nil, false, fmt.Errorf("parse workflow of job %d: %w", job.ID, err) - } else if len(gots) != 1 { + } else if len(parsedWorkflows) != 1 { return nil, false, fmt.Errorf("workflow of job %d: not single workflow", job.ID) - } else { //nolint:revive - _, workflowJob = gots[0].Job() } + _, workflowJob := parsedWorkflows[0].Job() if _, err := e.Insert(task); err != nil { return nil, false, err diff --git a/models/auth/auth_token.go b/models/auth/auth_token.go index 81f07d1a83..54ff5a0d75 100644 --- a/models/auth/auth_token.go +++ b/models/auth/auth_token.go @@ -15,7 +15,7 @@ import ( var ErrAuthTokenNotExist = util.NewNotExistErrorf("auth token does not exist") -type AuthToken struct { //nolint:revive +type AuthToken struct { //nolint:revive // export stutter ID string `xorm:"pk"` TokenHash string UserID int64 `xorm:"INDEX"` diff --git a/models/db/sql_postgres_with_schema.go b/models/db/sql_postgres_with_schema.go index 64b61b2ef3..812fe4a6a6 100644 --- a/models/db/sql_postgres_with_schema.go +++ b/models/db/sql_postgres_with_schema.go @@ -39,7 +39,7 @@ func (d *postgresSchemaDriver) Open(name string) (driver.Conn, error) { // golangci lint is incorrect here - there is no benefit to using driver.ExecerContext here // and in any case pq does not implement it - if execer, ok := conn.(driver.Execer); ok { //nolint:staticcheck + if execer, ok := conn.(driver.Execer); ok { //nolint:staticcheck // see above _, err := execer.Exec(`SELECT set_config( 'search_path', $1 || ',' || current_setting('search_path'), @@ -64,7 +64,7 @@ func (d *postgresSchemaDriver) Open(name string) (driver.Conn, error) { // driver.String.ConvertValue will never return err for string // golangci lint is incorrect here - there is no benefit to using stmt.ExecWithContext here - _, err = stmt.Exec([]driver.Value{schemaValue}) //nolint:staticcheck + _, err = stmt.Exec([]driver.Value{schemaValue}) //nolint:staticcheck // see above if err != nil { _ = conn.Close() return nil, err diff --git a/models/migrations/base/tests.go b/models/migrations/base/tests.go index 7da426fef0..33fd1df707 100644 --- a/models/migrations/base/tests.go +++ b/models/migrations/base/tests.go @@ -1,7 +1,6 @@ // Copyright 2022 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -//nolint:forbidigo package base import ( @@ -106,7 +105,7 @@ func MainTest(m *testing.M) { giteaConf := os.Getenv("GITEA_CONF") if giteaConf == "" { giteaConf = filepath.Join(filepath.Dir(setting.AppPath), "tests/sqlite.ini") - fmt.Printf("Environment variable $GITEA_CONF not set - defaulting to %s\n", giteaConf) + _, _ = fmt.Fprintf(os.Stderr, "Environment variable $GITEA_CONF not set - defaulting to %s\n", giteaConf) } if !filepath.IsAbs(giteaConf) { @@ -134,7 +133,7 @@ func MainTest(m *testing.M) { exitStatus := m.Run() if err := removeAllWithRetry(setting.RepoRootPath); err != nil { - fmt.Fprintf(os.Stderr, "os.RemoveAll: %v\n", err) + _, _ = fmt.Fprintf(os.Stderr, "os.RemoveAll: %v\n", err) } os.Exit(exitStatus) } diff --git a/models/migrations/v1_11/v112.go b/models/migrations/v1_11/v112.go index 2edc1c4bf0..fe45cf9222 100644 --- a/models/migrations/v1_11/v112.go +++ b/models/migrations/v1_11/v112.go @@ -4,9 +4,9 @@ package v1_11 import ( - "fmt" "path/filepath" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" @@ -31,7 +31,7 @@ func RemoveAttachmentMissedRepo(x *xorm.Engine) error { for i := 0; i < len(attachments); i++ { uuid := attachments[i].UUID if err = util.RemoveAll(filepath.Join(setting.Attachment.Storage.Path, uuid[0:1], uuid[1:2], uuid)); err != nil { - fmt.Printf("Error: %v", err) //nolint:forbidigo + log.Warn("Unable to remove attachment file by UUID %s: %v", uuid, err) } } diff --git a/models/migrations/v1_13/v140.go b/models/migrations/v1_13/v140.go index c8151df13e..a9a047bca9 100644 --- a/models/migrations/v1_13/v140.go +++ b/models/migrations/v1_13/v140.go @@ -21,12 +21,7 @@ func FixLanguageStatsToSaveSize(x *xorm.Engine) error { // RepoIndexerType specifies the repository indexer type type RepoIndexerType int - const ( - // RepoIndexerTypeCode code indexer - 0 - RepoIndexerTypeCode RepoIndexerType = iota //nolint:unused - // RepoIndexerTypeStats repository stats indexer - 1 - RepoIndexerTypeStats - ) + const RepoIndexerTypeStats RepoIndexerType = 1 // RepoIndexerStatus see models/repo_indexer.go type RepoIndexerStatus struct { diff --git a/models/migrations/v1_14/v157.go b/models/migrations/v1_14/v157.go index ba69f71130..2c5625ebbd 100644 --- a/models/migrations/v1_14/v157.go +++ b/models/migrations/v1_14/v157.go @@ -8,17 +8,6 @@ import ( ) func FixRepoTopics(x *xorm.Engine) error { - type Topic struct { //nolint:unused - ID int64 `xorm:"pk autoincr"` - Name string `xorm:"UNIQUE VARCHAR(25)"` - RepoCount int - } - - type RepoTopic struct { //nolint:unused - RepoID int64 `xorm:"pk"` - TopicID int64 `xorm:"pk"` - } - type Repository struct { ID int64 `xorm:"pk autoincr"` Topics []string `xorm:"TEXT JSON"` diff --git a/models/migrations/v1_14/v165.go b/models/migrations/v1_14/v165.go index 43b03c8bb7..6e1b34156b 100644 --- a/models/migrations/v1_14/v165.go +++ b/models/migrations/v1_14/v165.go @@ -16,10 +16,7 @@ func ConvertHookTaskTypeToVarcharAndTrim(x *xorm.Engine) error { return nil } - type HookTask struct { //nolint:unused - Typ string `xorm:"VARCHAR(16) index"` - } - + // HookTask: Typ string `xorm:"VARCHAR(16) index"` if err := base.ModifyColumn(x, "hook_task", &schemas.Column{ Name: "typ", SQLType: schemas.SQLType{ @@ -42,10 +39,7 @@ func ConvertHookTaskTypeToVarcharAndTrim(x *xorm.Engine) error { return err } - type Webhook struct { //nolint:unused - Type string `xorm:"VARCHAR(16) index"` - } - + // Webhook: Type string `xorm:"VARCHAR(16) index"` if err := base.ModifyColumn(x, "webhook", &schemas.Column{ Name: "type", SQLType: schemas.SQLType{ diff --git a/models/user/badge.go b/models/user/badge.go index 3ff3530a36..e475ceb748 100644 --- a/models/user/badge.go +++ b/models/user/badge.go @@ -19,7 +19,7 @@ type Badge struct { } // UserBadge represents a user badge -type UserBadge struct { //nolint:revive +type UserBadge struct { //nolint:revive // export stutter ID int64 `xorm:"pk autoincr"` BadgeID int64 UserID int64 `xorm:"INDEX"` diff --git a/modules/auth/password/hash/common.go b/modules/auth/password/hash/common.go index 487c0738f4..d5e2c34314 100644 --- a/modules/auth/password/hash/common.go +++ b/modules/auth/password/hash/common.go @@ -18,7 +18,7 @@ func parseIntParam(value, param, algorithmName, config string, previousErr error return parsed, previousErr // <- Keep the previous error as this function should still return an error once everything has been checked if any call failed } -func parseUIntParam(value, param, algorithmName, config string, previousErr error) (uint64, error) { //nolint:unparam +func parseUIntParam(value, param, algorithmName, config string, previousErr error) (uint64, error) { //nolint:unparam // algorithmName is always argon2 parsed, err := strconv.ParseUint(value, 10, 64) if err != nil { log.Error("invalid integer for %s representation in %s hash spec %s", param, algorithmName, config) diff --git a/modules/cache/cache_redis.go b/modules/cache/cache_redis.go index c5b52a2086..7473c938af 100644 --- a/modules/cache/cache_redis.go +++ b/modules/cache/cache_redis.go @@ -11,7 +11,7 @@ import ( "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/nosql" - "gitea.com/go-chi/cache" //nolint:depguard + "gitea.com/go-chi/cache" //nolint:depguard // we wrap this package here "github.com/redis/go-redis/v9" ) diff --git a/modules/cache/cache_twoqueue.go b/modules/cache/cache_twoqueue.go index 1eda2debc4..c8db686e57 100644 --- a/modules/cache/cache_twoqueue.go +++ b/modules/cache/cache_twoqueue.go @@ -10,7 +10,7 @@ import ( "code.gitea.io/gitea/modules/json" - mc "gitea.com/go-chi/cache" //nolint:depguard + mc "gitea.com/go-chi/cache" //nolint:depguard // we wrap this package here lru "github.com/hashicorp/golang-lru/v2" ) diff --git a/modules/cache/string_cache.go b/modules/cache/string_cache.go index 4f659616f5..3562b7a926 100644 --- a/modules/cache/string_cache.go +++ b/modules/cache/string_cache.go @@ -11,7 +11,7 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" - chi_cache "gitea.com/go-chi/cache" //nolint:depguard + chi_cache "gitea.com/go-chi/cache" //nolint:depguard // we wrap this package here ) type GetJSONError struct { diff --git a/modules/graceful/manager_windows.go b/modules/graceful/manager_windows.go index d776e0e9f9..457768d6ca 100644 --- a/modules/graceful/manager_windows.go +++ b/modules/graceful/manager_windows.go @@ -41,8 +41,7 @@ func (g *Manager) start() { // Make SVC process run := svc.Run - //lint:ignore SA1019 We use IsAnInteractiveSession because IsWindowsService has a different permissions profile - isAnInteractiveSession, err := svc.IsAnInteractiveSession() //nolint:staticcheck + isAnInteractiveSession, err := svc.IsAnInteractiveSession() //nolint:staticcheck // must use IsAnInteractiveSession because IsWindowsService has a different permissions profile if err != nil { log.Error("Unable to ascertain if running as an Windows Service: %v", err) return diff --git a/modules/json/json.go b/modules/json/json.go index acd4118573..444dc8526a 100644 --- a/modules/json/json.go +++ b/modules/json/json.go @@ -3,11 +3,10 @@ package json -// Allow "encoding/json" import. import ( "bytes" "encoding/binary" - "encoding/json" //nolint:depguard + "encoding/json" //nolint:depguard // this package wraps it "io" jsoniter "github.com/json-iterator/go" diff --git a/modules/log/logger.go b/modules/log/logger.go index 3fc524d55e..8b89e0eb5a 100644 --- a/modules/log/logger.go +++ b/modules/log/logger.go @@ -45,6 +45,6 @@ type Logger interface { LevelLogger } -type LogStringer interface { //nolint:revive +type LogStringer interface { //nolint:revive // export stutter LogString() string } diff --git a/modules/markup/markdown/transform_blockquote.go b/modules/markup/markdown/transform_blockquote.go index 3a8c6fa018..bf17f01681 100644 --- a/modules/markup/markdown/transform_blockquote.go +++ b/modules/markup/markdown/transform_blockquote.go @@ -46,7 +46,7 @@ func (g *ASTTransformer) extractBlockquoteAttentionEmphasis(firstParagraph ast.N if !ok { return "", nil } - val1 := string(node1.Text(reader.Source())) //nolint:staticcheck + val1 := string(node1.Text(reader.Source())) //nolint:staticcheck // Text is deprecated attentionType := strings.ToLower(val1) if g.attentionTypes.Contains(attentionType) { return attentionType, []ast.Node{node1} diff --git a/modules/markup/markdown/transform_codespan.go b/modules/markup/markdown/transform_codespan.go index bccc43aad2..c2e4295bc2 100644 --- a/modules/markup/markdown/transform_codespan.go +++ b/modules/markup/markdown/transform_codespan.go @@ -68,7 +68,7 @@ func cssColorHandler(value string) bool { } func (g *ASTTransformer) transformCodeSpan(_ *markup.RenderContext, v *ast.CodeSpan, reader text.Reader) { - colorContent := v.Text(reader.Source()) //nolint:staticcheck + colorContent := v.Text(reader.Source()) //nolint:staticcheck // Text is deprecated if cssColorHandler(string(colorContent)) { v.AppendChild(v, NewColorPreview(colorContent)) } diff --git a/modules/markup/markdown/transform_heading.go b/modules/markup/markdown/transform_heading.go index cac3cd6617..a229a7b1a4 100644 --- a/modules/markup/markdown/transform_heading.go +++ b/modules/markup/markdown/transform_heading.go @@ -19,7 +19,7 @@ func (g *ASTTransformer) transformHeading(_ *markup.RenderContext, v *ast.Headin v.SetAttribute(attr.Name, fmt.Appendf(nil, "%v", attr.Value)) } } - txt := v.Text(reader.Source()) //nolint:staticcheck + txt := v.Text(reader.Source()) //nolint:staticcheck // Text is deprecated header := Header{ Text: util.UnsafeBytesToString(txt), Level: v.Level, diff --git a/modules/markup/mdstripper/mdstripper.go b/modules/markup/mdstripper/mdstripper.go index c589926b5e..6e392444b4 100644 --- a/modules/markup/mdstripper/mdstripper.go +++ b/modules/markup/mdstripper/mdstripper.go @@ -46,7 +46,7 @@ func (r *stripRenderer) Render(w io.Writer, source []byte, doc ast.Node) error { coalesce := prevSibIsText r.processString( w, - v.Text(source), //nolint:staticcheck + v.Text(source), //nolint:staticcheck // Text is deprecated coalesce) if v.SoftLineBreak() { r.doubleSpace(w) diff --git a/modules/optional/serialization_test.go b/modules/optional/serialization_test.go index 21d3ad8470..cf81a94cfc 100644 --- a/modules/optional/serialization_test.go +++ b/modules/optional/serialization_test.go @@ -4,7 +4,7 @@ package optional_test import ( - std_json "encoding/json" //nolint:depguard + std_json "encoding/json" //nolint:depguard // for testing purpose "testing" "code.gitea.io/gitea/modules/json" diff --git a/modules/setting/config_env.go b/modules/setting/config_env.go index 5d94a9641f..409588dc44 100644 --- a/modules/setting/config_env.go +++ b/modules/setting/config_env.go @@ -97,7 +97,7 @@ func decodeEnvSectionKey(encoded string) (ok bool, section, key string) { // decodeEnvironmentKey decode the environment key to section and key // The environment key is in the form of GITEA__SECTION__KEY or GITEA__SECTION__KEY__FILE -func decodeEnvironmentKey(prefixGitea, suffixFile, envKey string) (ok bool, section, key string, useFileValue bool) { //nolint:unparam +func decodeEnvironmentKey(prefixGitea, suffixFile, envKey string) (ok bool, section, key string, useFileValue bool) { if !strings.HasPrefix(envKey, prefixGitea) { return false, "", "", false } diff --git a/modules/setting/config_env_test.go b/modules/setting/config_env_test.go index 217ea53860..7d270ac21a 100644 --- a/modules/setting/config_env_test.go +++ b/modules/setting/config_env_test.go @@ -73,6 +73,9 @@ func TestDecodeEnvironmentKey(t *testing.T) { assert.Equal(t, "sec", section) assert.Equal(t, "KEY", key) assert.True(t, file) + + ok, _, _, _ = decodeEnvironmentKey("PREFIX__", "", "PREFIX__SEC__KEY") + assert.True(t, ok) } func TestEnvironmentToConfig(t *testing.T) { diff --git a/modules/setting/config_provider.go b/modules/setting/config_provider.go index a0c53a1032..09eaaefdaf 100644 --- a/modules/setting/config_provider.go +++ b/modules/setting/config_provider.go @@ -15,7 +15,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/util" - "gopkg.in/ini.v1" //nolint:depguard + "gopkg.in/ini.v1" //nolint:depguard // wrapper for this package ) type ConfigKey interface { diff --git a/modules/setting/security.go b/modules/setting/security.go index 3ae4c005c7..153b6bc944 100644 --- a/modules/setting/security.go +++ b/modules/setting/security.go @@ -111,7 +111,7 @@ func loadSecurityFrom(rootCfg ConfigProvider) { if SecretKey == "" { // FIXME: https://github.com/go-gitea/gitea/issues/16832 // Until it supports rotating an existing secret key, we shouldn't move users off of the widely used default value - SecretKey = "!#@FDEWREWR&*(" //nolint:gosec + SecretKey = "!#@FDEWREWR&*(" } CookieRememberName = sec.Key("COOKIE_REMEMBER_NAME").MustString("gitea_incredible") diff --git a/modules/setting/storage.go b/modules/setting/storage.go index f43af1a8c0..ee246158d9 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -158,7 +158,7 @@ const ( targetSecIsSec // target section is from the name seciont [name] ) -func getStorageSectionByType(rootCfg ConfigProvider, typ string) (ConfigSection, targetSecType, error) { //nolint:unparam +func getStorageSectionByType(rootCfg ConfigProvider, typ string) (ConfigSection, targetSecType, error) { //nolint:unparam // FIXME: targetSecType is always 0, wrong design? targetSec, err := rootCfg.GetSection(storageSectionName + "." + typ) if err != nil { if !IsValidStorageType(StorageType(typ)) { @@ -283,7 +283,7 @@ func getStorageForLocal(targetSec, overrideSec ConfigSection, tp targetSecType, return &storage, nil } -func getStorageForMinio(targetSec, overrideSec ConfigSection, tp targetSecType, name string) (*Storage, error) { //nolint:dupl +func getStorageForMinio(targetSec, overrideSec ConfigSection, tp targetSecType, name string) (*Storage, error) { //nolint:dupl // duplicates azure setup var storage Storage storage.Type = StorageType(targetSec.Key("STORAGE_TYPE").String()) if err := targetSec.MapTo(&storage.MinioConfig); err != nil { @@ -312,7 +312,7 @@ func getStorageForMinio(targetSec, overrideSec ConfigSection, tp targetSecType, return &storage, nil } -func getStorageForAzureBlob(targetSec, overrideSec ConfigSection, tp targetSecType, name string) (*Storage, error) { //nolint:dupl +func getStorageForAzureBlob(targetSec, overrideSec ConfigSection, tp targetSecType, name string) (*Storage, error) { //nolint:dupl // duplicates minio setup var storage Storage storage.Type = StorageType(targetSec.Key("STORAGE_TYPE").String()) if err := targetSec.MapTo(&storage.AzureBlobConfig); err != nil { diff --git a/modules/templates/htmlrenderer.go b/modules/templates/htmlrenderer.go index f51936354e..8073a6e5f5 100644 --- a/modules/templates/htmlrenderer.go +++ b/modules/templates/htmlrenderer.go @@ -42,7 +42,7 @@ var ( var ErrTemplateNotInitialized = errors.New("template system is not initialized, check your log for errors") -func (h *HTMLRender) HTML(w io.Writer, status int, tplName TplName, data any, ctx context.Context) error { //nolint:revive +func (h *HTMLRender) HTML(w io.Writer, status int, tplName TplName, data any, ctx context.Context) error { //nolint:revive // we don't use ctx, only pass it to the template executor name := string(tplName) if respWriter, ok := w.(http.ResponseWriter); ok { if respWriter.Header().Get("Content-Type") == "" { @@ -57,7 +57,7 @@ func (h *HTMLRender) HTML(w io.Writer, status int, tplName TplName, data any, ct return t.Execute(w, data) } -func (h *HTMLRender) TemplateLookup(name string, ctx context.Context) (TemplateExecutor, error) { //nolint:revive +func (h *HTMLRender) TemplateLookup(name string, ctx context.Context) (TemplateExecutor, error) { //nolint:revive // we don't use ctx, only pass it to the template executor tmpls := h.templates.Load() if tmpls == nil { return nil, ErrTemplateNotInitialized diff --git a/modules/templates/scopedtmpl/scopedtmpl.go b/modules/templates/scopedtmpl/scopedtmpl.go index 0d84f8598b..34e8b9ad70 100644 --- a/modules/templates/scopedtmpl/scopedtmpl.go +++ b/modules/templates/scopedtmpl/scopedtmpl.go @@ -102,31 +102,28 @@ func escapeTemplate(t *template.Template) error { return nil } -//nolint:unused type htmlTemplate struct { - escapeErr error - text *texttemplate.Template + _/*escapeErr*/ error + text *texttemplate.Template } -//nolint:unused type textTemplateCommon struct { - tmpl map[string]*template.Template // Map from name to defined templates. - muTmpl sync.RWMutex // protects tmpl - option struct { + _/*tmpl*/ map[string]*template.Template + _/*muTmpl*/ sync.RWMutex + _/*option*/ struct { missingKey int } - muFuncs sync.RWMutex // protects parseFuncs and execFuncs - parseFuncs texttemplate.FuncMap - execFuncs map[string]reflect.Value + muFuncs sync.RWMutex + _/*parseFuncs*/ texttemplate.FuncMap + execFuncs map[string]reflect.Value } -//nolint:unused type textTemplate struct { - name string + _/*name*/ string *parse.Tree *textTemplateCommon - leftDelim string - rightDelim string + _/*leftDelim*/ string + _/*rightDelim*/ string } func ptr[T, P any](ptr *P) *T { diff --git a/routers/api/actions/artifacts_utils.go b/routers/api/actions/artifacts_utils.go index 77ce765098..35868c290e 100644 --- a/routers/api/actions/artifacts_utils.go +++ b/routers/api/actions/artifacts_utils.go @@ -43,7 +43,7 @@ func validateRunID(ctx *ArtifactContext) (*actions.ActionTask, int64, bool) { return task, runID, true } -func validateRunIDV4(ctx *ArtifactContext, rawRunID string) (*actions.ActionTask, int64, bool) { //nolint:unparam +func validateRunIDV4(ctx *ArtifactContext, rawRunID string) (*actions.ActionTask, int64, bool) { //nolint:unparam // ActionTask is never used task := ctx.ActionTask runID, err := strconv.ParseInt(rawRunID, 10, 64) if err != nil || task.Job.RunID != runID { diff --git a/routers/api/packages/container/blob.go b/routers/api/packages/container/blob.go index abfc21f95a..7028cfb40b 100644 --- a/routers/api/packages/container/blob.go +++ b/routers/api/packages/container/blob.go @@ -26,7 +26,7 @@ import ( // saveAsPackageBlob creates a package blob from an upload // The uploaded blob gets stored in a special upload version to link them to the package/image -func saveAsPackageBlob(ctx context.Context, hsr packages_module.HashedSizeReader, pci *packages_service.PackageCreationInfo) (*packages_model.PackageBlob, error) { //nolint:unparam +func saveAsPackageBlob(ctx context.Context, hsr packages_module.HashedSizeReader, pci *packages_service.PackageCreationInfo) (*packages_model.PackageBlob, error) { //nolint:unparam // PackageBlob is never used pb := packages_service.NewPackageBlob(hsr) exists := false diff --git a/routers/api/packages/nuget/nuget.go b/routers/api/packages/nuget/nuget.go index 0a3254979d..92d62d90b1 100644 --- a/routers/api/packages/nuget/nuget.go +++ b/routers/api/packages/nuget/nuget.go @@ -36,7 +36,7 @@ func apiError(ctx *context.Context, status int, obj any) { }) } -func xmlResponse(ctx *context.Context, status int, obj any) { //nolint:unparam +func xmlResponse(ctx *context.Context, status int, obj any) { //nolint:unparam // status is always StatusOK ctx.Resp.Header().Set("Content-Type", "application/atom+xml; charset=utf-8") ctx.Resp.WriteHeader(status) if _, err := ctx.Resp.Write([]byte(xml.Header)); err != nil { diff --git a/routers/api/v1/misc/markup.go b/routers/api/v1/misc/markup.go index 0cd4b8c5c5..909310b4c8 100644 --- a/routers/api/v1/misc/markup.go +++ b/routers/api/v1/misc/markup.go @@ -42,7 +42,7 @@ func Markup(ctx *context.APIContext) { return } - mode := util.Iif(form.Wiki, "wiki", form.Mode) //nolint:staticcheck + mode := util.Iif(form.Wiki, "wiki", form.Mode) //nolint:staticcheck // form.Wiki is deprecated common.RenderMarkup(ctx.Base, ctx.Repo, mode, form.Text, form.Context, form.FilePath) } @@ -73,7 +73,7 @@ func Markdown(ctx *context.APIContext) { return } - mode := util.Iif(form.Wiki, "wiki", form.Mode) //nolint:staticcheck + mode := util.Iif(form.Wiki, "wiki", form.Mode) //nolint:staticcheck // form.Wiki is deprecated common.RenderMarkup(ctx.Base, ctx.Repo, mode, form.Text, form.Context, "") } diff --git a/routers/web/admin/config.go b/routers/web/admin/config.go index 5387d2b26f..0e5b23db6d 100644 --- a/routers/web/admin/config.go +++ b/routers/web/admin/config.go @@ -200,7 +200,7 @@ func ChangeConfig(ctx *context.Context) { value := ctx.FormString("value") cfg := setting.Config() - marshalBool := func(v string) (string, error) { //nolint:unparam + marshalBool := func(v string) (string, error) { //nolint:unparam // error is always nil if b, _ := strconv.ParseBool(v); b { return "true", nil } diff --git a/routers/web/misc/markup.go b/routers/web/misc/markup.go index 0c7ec6c2eb..f90cf3ffed 100644 --- a/routers/web/misc/markup.go +++ b/routers/web/misc/markup.go @@ -15,6 +15,6 @@ import ( // Markup render markup document to HTML func Markup(ctx *context.Context) { form := web.GetForm(ctx).(*api.MarkupOption) - mode := util.Iif(form.Wiki, "wiki", form.Mode) //nolint:staticcheck + mode := util.Iif(form.Wiki, "wiki", form.Mode) //nolint:staticcheck // form.Wiki is deprecated common.RenderMarkup(ctx.Base, ctx.Repo, mode, form.Text, form.Context, form.FilePath) } diff --git a/routers/web/web.go b/routers/web/web.go index 66a5a9be5e..1039f9e739 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1031,7 +1031,7 @@ func registerWebRoutes(m *web.Router) { m.Get("", org.Projects) m.Get("/{id}", org.ViewProject) }, reqUnitAccess(unit.TypeProjects, perm.AccessModeRead, true)) - m.Group("", func() { //nolint:dupl + m.Group("", func() { //nolint:dupl // duplicates lines 1421-1441 m.Get("/new", org.RenderNewProject) m.Post("/new", web.Bind(forms.CreateProjectForm{}), org.NewProjectPost) m.Group("/{id}", func() { @@ -1418,7 +1418,7 @@ func registerWebRoutes(m *web.Router) { m.Group("/{username}/{reponame}/projects", func() { m.Get("", repo.Projects) m.Get("/{id}", repo.ViewProject) - m.Group("", func() { //nolint:dupl + m.Group("", func() { //nolint:dupl // duplicates lines 1034-1054 m.Get("/new", repo.RenderNewProject) m.Post("/new", web.Bind(forms.CreateProjectForm{}), repo.NewProjectPost) m.Group("/{id}", func() { diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index 3180fbf0a1..8010f51a86 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -33,7 +33,9 @@ import ( "github.com/nektos/act/pkg/model" ) -var methodCtxKey struct{} +type methodCtxKeyType struct{} + +var methodCtxKey methodCtxKeyType // withMethod sets the notification method that this context currently executes. // Used for debugging/ troubleshooting purposes. @@ -44,8 +46,7 @@ func withMethod(ctx context.Context, method string) context.Context { return ctx } } - // FIXME: review the use of this nolint directive - return context.WithValue(ctx, methodCtxKey, method) //nolint:staticcheck + return context.WithValue(ctx, methodCtxKey, method) } // getMethod gets the notification method that this context currently executes. diff --git a/services/auth/source/ldap/source_search.go b/services/auth/source/ldap/source_search.go index fa2c45ce4a..e6bce04a83 100644 --- a/services/auth/source/ldap/source_search.go +++ b/services/auth/source/ldap/source_search.go @@ -117,10 +117,10 @@ func dial(source *Source) (*ldap.Conn, error) { } if source.SecurityProtocol == SecurityProtocolLDAPS { - return ldap.DialTLS("tcp", net.JoinHostPort(source.Host, strconv.Itoa(source.Port)), tlsConfig) //nolint:staticcheck + return ldap.DialTLS("tcp", net.JoinHostPort(source.Host, strconv.Itoa(source.Port)), tlsConfig) //nolint:staticcheck // DialTLS is deprecated } - conn, err := ldap.Dial("tcp", net.JoinHostPort(source.Host, strconv.Itoa(source.Port))) //nolint:staticcheck + conn, err := ldap.Dial("tcp", net.JoinHostPort(source.Host, strconv.Itoa(source.Port))) //nolint:staticcheck // Dial is deprecated if err != nil { return nil, fmt.Errorf("error during Dial: %w", err) } diff --git a/services/doctor/paths.go b/services/doctor/paths.go index 3f62d587ab..4214c36b1a 100644 --- a/services/doctor/paths.go +++ b/services/doctor/paths.go @@ -99,15 +99,14 @@ func checkConfigurationFiles(ctx context.Context, logger log.Logger, autofix boo func isWritableDir(path string) error { // There's no platform-independent way of checking if a directory is writable // https://stackoverflow.com/questions/20026320/how-to-tell-if-folder-exists-and-is-writable - tmpFile, err := os.CreateTemp(path, "doctors-order") if err != nil { return err } if err := os.Remove(tmpFile.Name()); err != nil { - fmt.Printf("Warning: can't remove temporary file: '%s'\n", tmpFile.Name()) //nolint:forbidigo + log.Warn("can't remove temporary file: %q", tmpFile.Name()) } - tmpFile.Close() + _ = tmpFile.Close() return nil } diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index f85b13e97f..9964329876 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -214,51 +214,6 @@ func (diffSection *DiffSection) GetLine(idx int) *DiffLine { return diffSection.Lines[idx] } -// GetLine gets a specific line by type (add or del) and file line number -// This algorithm is not quite right. -// Actually now we have "Match" field, it is always right, so use it instead in new GetLine -func (diffSection *DiffSection) getLineLegacy(lineType DiffLineType, idx int) *DiffLine { //nolint:unused - var ( - difference = 0 - addCount = 0 - delCount = 0 - matchDiffLine *DiffLine - ) - -LOOP: - for _, diffLine := range diffSection.Lines { - switch diffLine.Type { - case DiffLineAdd: - addCount++ - case DiffLineDel: - delCount++ - default: - if matchDiffLine != nil { - break LOOP - } - difference = diffLine.RightIdx - diffLine.LeftIdx - addCount = 0 - delCount = 0 - } - - switch lineType { - case DiffLineDel: - if diffLine.RightIdx == 0 && diffLine.LeftIdx == idx-difference { - matchDiffLine = diffLine - } - case DiffLineAdd: - if diffLine.LeftIdx == 0 && diffLine.RightIdx == idx+difference { - matchDiffLine = diffLine - } - } - } - - if addCount == delCount { - return matchDiffLine - } - return nil -} - func defaultDiffMatchPatch() *diffmatchpatch.DiffMatchPatch { dmp := diffmatchpatch.New() dmp.DiffEditCost = 100 @@ -1348,15 +1303,14 @@ func SyncUserSpecificDiff(ctx context.Context, userID int64, pull *issues_model. latestCommit = pull.HeadBranch // opts.AfterCommitID is preferred because it handles PRs from forks correctly and the branch name doesn't } - changedFiles, err := gitRepo.GetFilesChangedBetween(review.CommitSHA, latestCommit) + changedFiles, errIgnored := gitRepo.GetFilesChangedBetween(review.CommitSHA, latestCommit) // There are way too many possible errors. // Examples are various git errors such as the commit the review was based on was gc'ed and hence doesn't exist anymore as well as unrecoverable errors where we should serve a 500 response // Due to the current architecture and physical limitation of needing to compare explicit error messages, we can only choose one approach without the code getting ugly // For SOME of the errors such as the gc'ed commit, it would be best to mark all files as changed // But as that does not work for all potential errors, we simply mark all files as unchanged and drop the error which always works, even if not as good as possible - if err != nil { + if errIgnored != nil { log.Error("Could not get changed files between %s and %s for pull request %d in repo with path %s. Assuming no changes. Error: %w", review.CommitSHA, latestCommit, pull.Index, gitRepo.Path, err) - err = nil //nolint:ineffassign,wastedassign } filesChangedSinceLastDiff := make(map[string]pull_model.ViewedState) diff --git a/services/migrations/migrate.go b/services/migrations/migrate.go index 15458e761c..eba9c79df5 100644 --- a/services/migrations/migrate.go +++ b/services/migrations/migrate.go @@ -75,11 +75,9 @@ func IsMigrateURLAllowed(remoteURL string, doer *user_model.User) error { return &git.ErrInvalidCloneAddr{Host: u.Host, IsProtocolInvalid: true, IsPermissionDenied: true, IsURLError: true} } - hostName, _, err := net.SplitHostPort(u.Host) - if err != nil { - // u.Host can be "host" or "host:port" - err = nil //nolint:ineffassign,wastedassign - hostName = u.Host + hostName, _, errIgnored := net.SplitHostPort(u.Host) + if errIgnored != nil { + hostName = u.Host // u.Host can be "host" or "host:port" } // some users only use proxy, there is no DNS resolver. it's safe to ignore the LookupIP error diff --git a/services/packages/rpm/repository.go b/services/packages/rpm/repository.go index 5027021c52..fbbf8d7dad 100644 --- a/services/packages/rpm/repository.go +++ b/services/packages/rpm/repository.go @@ -470,7 +470,7 @@ func buildPrimary(ctx context.Context, pv *packages_model.PackageVersion, pfs [] } // https://docs.pulpproject.org/en/2.19/plugins/pulp_rpm/tech-reference/rpm.html#filelists-xml -func buildFilelists(ctx context.Context, pv *packages_model.PackageVersion, pfs []*packages_model.PackageFile, c packageCache, group string) (*repoData, error) { //nolint:dupl +func buildFilelists(ctx context.Context, pv *packages_model.PackageVersion, pfs []*packages_model.PackageFile, c packageCache, group string) (*repoData, error) { //nolint:dupl // duplicates with buildOther type Version struct { Epoch string `xml:"epoch,attr"` Version string `xml:"ver,attr"` @@ -517,7 +517,7 @@ func buildFilelists(ctx context.Context, pv *packages_model.PackageVersion, pfs } // https://docs.pulpproject.org/en/2.19/plugins/pulp_rpm/tech-reference/rpm.html#other-xml -func buildOther(ctx context.Context, pv *packages_model.PackageVersion, pfs []*packages_model.PackageFile, c packageCache, group string) (*repoData, error) { //nolint:dupl +func buildOther(ctx context.Context, pv *packages_model.PackageVersion, pfs []*packages_model.PackageFile, c packageCache, group string) (*repoData, error) { //nolint:dupl // duplicates with buildFilelists type Version struct { Epoch string `xml:"epoch,attr"` Version string `xml:"ver,attr"` diff --git a/services/pull/merge.go b/services/pull/merge.go index 7a74bf55ae..cd9aeb2ad1 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -325,7 +325,7 @@ func handleCloseCrossReferences(ctx context.Context, pr *issues_model.PullReques } // doMergeAndPush performs the merge operation without changing any pull information in database and pushes it up to the base repository -func doMergeAndPush(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string, pushTrigger repo_module.PushTrigger) (string, error) { //nolint:unparam +func doMergeAndPush(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string, pushTrigger repo_module.PushTrigger) (string, error) { //nolint:unparam // non-error result is never used // Clone base repo. mergeCtx, cancel, err := createTemporaryRepoForMerge(ctx, pr, doer, expectedHeadCommitID) if err != nil { diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index af631ca8fd..4a408dbd7b 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -4,7 +4,7 @@ // This is primarily coped from /tests/integration/integration_test.go // TODO: Move common functions to shared file -//nolint:forbidigo +//nolint:forbidigo // use of print functions is allowed in tests package e2e import ( diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index d5b7bb7a3e..21126b63c5 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -1,7 +1,7 @@ // Copyright 2017 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -//nolint:forbidigo +//nolint:forbidigo // use of print functions is allowed in tests package integration import ( diff --git a/tests/test_utils.go b/tests/test_utils.go index 4d8a8635d6..ad692058a6 100644 --- a/tests/test_utils.go +++ b/tests/test_utils.go @@ -1,7 +1,6 @@ // Copyright 2017 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -//nolint:forbidigo package tests import ( @@ -55,7 +54,7 @@ func InitTest(requireGitea bool) { // Notice: when doing "ssh push", Gitea executes sub processes, debugger won't work for the sub processes. giteaConf = "tests/sqlite.ini" _ = os.Setenv("GITEA_CONF", giteaConf) - fmt.Printf("Environment variable $GITEA_CONF not set, use default: %s\n", giteaConf) + _, _ = fmt.Fprintf(os.Stderr, "Environment variable $GITEA_CONF not set - defaulting to %s\n", giteaConf) if !setting.EnableSQLite3 { testlogger.Fatalf(`sqlite3 requires: -tags sqlite,sqlite_unlock_notify` + "\n") } From 1e50cec0b3109ff06be5c736d2b4345e9bd5646f Mon Sep 17 00:00:00 2001 From: silverwind Date: Fri, 27 Jun 2025 17:12:25 +0200 Subject: [PATCH 22/62] Improve `labels-list` rendering (#34846) Make labels list use consistent gap --------- Co-authored-by: wxiaoguang --- modules/htmlutil/html.go | 2 + modules/htmlutil/html_test.go | 9 +++ modules/templates/util_render.go | 41 +++++++--- modules/templates/util_render_test.go | 11 +++ templates/repo/home_sidebar_bottom.tmpl | 4 +- templates/repo/issue/card.tmpl | 15 ++-- templates/repo/issue/labels/label_list.tmpl | 6 +- .../repo/issue/sidebar/assignee_list.tmpl | 2 +- templates/repo/issue/sidebar/label_list.tmpl | 2 +- .../repo/issue/sidebar/reviewer_list.tmpl | 2 +- .../repo/issue/view_content/comments.tmpl | 2 +- templates/shared/issuelist.tmpl | 17 +++-- web_src/css/base.css | 11 ++- web_src/css/modules/label.css | 75 +++++++++++++++---- web_src/css/modules/list.css | 1 - web_src/css/repo.css | 54 +++---------- web_src/css/repo/issue-card.css | 9 ++- web_src/css/repo/issue-label.css | 25 ++++--- web_src/css/shared/flex-list.css | 12 +-- 19 files changed, 185 insertions(+), 115 deletions(-) diff --git a/modules/htmlutil/html.go b/modules/htmlutil/html.go index 194135ba18..efbc174b2e 100644 --- a/modules/htmlutil/html.go +++ b/modules/htmlutil/html.go @@ -42,6 +42,8 @@ func HTMLFormat(s template.HTML, rawArgs ...any) template.HTML { // for most basic types (including template.HTML which is safe), just do nothing and use it case string: args[i] = template.HTMLEscapeString(v) + case template.URL: + args[i] = template.HTMLEscapeString(string(v)) case fmt.Stringer: args[i] = template.HTMLEscapeString(v.String()) default: diff --git a/modules/htmlutil/html_test.go b/modules/htmlutil/html_test.go index 5ff05d75b3..22258ce59d 100644 --- a/modules/htmlutil/html_test.go +++ b/modules/htmlutil/html_test.go @@ -10,6 +10,15 @@ import ( "github.com/stretchr/testify/assert" ) +type testStringer struct{} + +func (t testStringer) String() string { + return "&StringMethod" +} + func TestHTMLFormat(t *testing.T) { assert.Equal(t, template.HTML("< < 1"), HTMLFormat("%s %s %d", "<", template.HTML("<"), 1)) + assert.Equal(t, template.HTML("%!s()"), HTMLFormat("%s", nil)) + assert.Equal(t, template.HTML("<>"), HTMLFormat("%s", template.URL("<>"))) + assert.Equal(t, template.HTML("&StringMethod &StringMethod"), HTMLFormat("%s %s", testStringer{}, &testStringer{})) } diff --git a/modules/templates/util_render.go b/modules/templates/util_render.go index 377e4487ec..1056c42643 100644 --- a/modules/templates/util_render.go +++ b/modules/templates/util_render.go @@ -122,8 +122,23 @@ func (ut *RenderUtils) RenderIssueSimpleTitle(text string) template.HTML { return ret } -// RenderLabel renders a label +func (ut *RenderUtils) RenderLabelWithLink(label *issues_model.Label, link any) template.HTML { + var attrHref template.HTML + switch link.(type) { + case template.URL, string: + attrHref = htmlutil.HTMLFormat(`href="%s"`, link) + default: + panic(fmt.Sprintf("unexpected type %T for link", link)) + } + return ut.renderLabelWithTag(label, "a", attrHref) +} + func (ut *RenderUtils) RenderLabel(label *issues_model.Label) template.HTML { + return ut.renderLabelWithTag(label, "span", "") +} + +// RenderLabel renders a label +func (ut *RenderUtils) renderLabelWithTag(label *issues_model.Label, tagName, tagAttrs template.HTML) template.HTML { locale := ut.ctx.Value(translation.ContextKey).(translation.Locale) var extraCSSClasses string textColor := util.ContrastColor(label.Color) @@ -137,8 +152,8 @@ func (ut *RenderUtils) RenderLabel(label *issues_model.Label) template.HTML { if labelScope == "" { // Regular label - return htmlutil.HTMLFormat(`
%s
`, - extraCSSClasses, textColor, label.Color, descriptionText, ut.RenderEmoji(label.Name)) + return htmlutil.HTMLFormat(`<%s %s class="ui label %s" style="color: %s !important; background-color: %s !important;" data-tooltip-content title="%s">%s`, + tagName, tagAttrs, extraCSSClasses, textColor, label.Color, descriptionText, ut.RenderEmoji(label.Name), tagName) } // Scoped label @@ -152,7 +167,7 @@ func (ut *RenderUtils) RenderLabel(label *issues_model.Label) template.HTML { // Ensure we add the same amount of contrast also near 0 and 1. darken := contrast + math.Max(luminance+contrast-1.0, 0.0) lighten := contrast + math.Max(contrast-luminance, 0.0) - // Compute factor to keep RGB values proportional. + // Compute the factor to keep RGB values proportional. darkenFactor := math.Max(luminance-darken, 0.0) / math.Max(luminance, 1.0/255.0) lightenFactor := math.Min(luminance+lighten, 1.0) / math.Max(luminance, 1.0/255.0) @@ -173,26 +188,29 @@ func (ut *RenderUtils) RenderLabel(label *issues_model.Label) template.HTML { if label.ExclusiveOrder > 0 { // |
- - {{range $idx, $release := .Releases}} - - - - {{end}} - -
-

- {{if $canReadReleases}} - {{.TagName}} - {{else}} - {{.TagName}} - {{end}} -

-
- {{if $.Permission.CanRead ctx.Consts.RepoUnitTypeCode}} - {{if .CreatedUnix}} - {{svg "octicon-clock" 16 "tw-mr-1"}}{{DateUtils.TimeSince .CreatedUnix}} - {{end}} +
+ {{range $idx, $release := .Releases}} +
+

+ {{if $canReadReleases}} + {{.TagName}} + {{else}} + {{.TagName}} + {{end}} +

+ -
+ {{if and $canReadReleases (not $release.IsTag)}} + {{svg "octicon-tag"}}{{ctx.Locale.Tr "repo.release.detail"}} + {{end}} + {{end}} +
+ + {{end}} + {{else}} {{if .NumTags}}

{{ctx.Locale.Tr "no_results_found"}}

@@ -73,9 +67,8 @@ {{if $.Permission.CanWrite ctx.Consts.RepoUnitTypeCode}} - - - @@ -95,7 +95,7 @@ {{if $allowedToChangeTeams}}
-
@@ -123,14 +123,9 @@ {{end}} -
-
@@ -72,14 +72,9 @@ - @@ -96,14 +96,9 @@ - {{svg "octicon-pencil"}} - {{svg "octicon-trash"}} + {{svg "octicon-trash"}} {{end}} diff --git a/templates/repo/settings/webhook/delete_modal.tmpl b/templates/repo/settings/webhook/delete_modal.tmpl deleted file mode 100644 index 9955ed3a2f..0000000000 --- a/templates/repo/settings/webhook/delete_modal.tmpl +++ /dev/null @@ -1,10 +0,0 @@ - diff --git a/templates/repo/settings/webhook/dingtalk.tmpl b/templates/repo/settings/webhook/dingtalk.tmpl index 0ba99e98ee..dd208cde17 100644 --- a/templates/repo/settings/webhook/dingtalk.tmpl +++ b/templates/repo/settings/webhook/dingtalk.tmpl @@ -6,6 +6,7 @@ - {{template "repo/settings/webhook/settings" .}} + {{/* FIXME: support authorization header or not? */}} + {{template "repo/settings/webhook/settings" dict "BaseLink" .BaseLink "Webhook" .Webhook "UseAuthorizationHeader" "optional"}} {{end}} diff --git a/templates/repo/settings/webhook/discord.tmpl b/templates/repo/settings/webhook/discord.tmpl index 104346e042..fa66249fa5 100644 --- a/templates/repo/settings/webhook/discord.tmpl +++ b/templates/repo/settings/webhook/discord.tmpl @@ -14,6 +14,7 @@ - {{template "repo/settings/webhook/settings" .}} + {{/* FIXME: support authorization header or not? */}} + {{template "repo/settings/webhook/settings" dict "BaseLink" .BaseLink "Webhook" .Webhook "UseAuthorizationHeader" "optional"}} {{end}} diff --git a/templates/repo/settings/webhook/feishu.tmpl b/templates/repo/settings/webhook/feishu.tmpl index d80deab26f..13bd0d92a1 100644 --- a/templates/repo/settings/webhook/feishu.tmpl +++ b/templates/repo/settings/webhook/feishu.tmpl @@ -1,12 +1,14 @@ {{if eq .HookType "feishu"}} -

{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://feishu.cn" (ctx.Locale.Tr "repo.settings.web_hook_name_feishu")}}

-

{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://larksuite.com" (ctx.Locale.Tr "repo.settings.web_hook_name_larksuite")}}

+

+ {{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://feishu.cn" (ctx.Locale.Tr "repo.settings.web_hook_name_feishu")}} + {{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://larksuite.com" (ctx.Locale.Tr "repo.settings.web_hook_name_larksuite")}} +

{{.CsrfTokenHtml}}
- {{template "repo/settings/webhook/settings" .}} + {{template "repo/settings/webhook/settings" dict "BaseLink" .BaseLink "Webhook" .Webhook "UseRequestSecret" "optional"}}
{{end}} diff --git a/templates/repo/settings/webhook/gitea.tmpl b/templates/repo/settings/webhook/gitea.tmpl index e6eb61ea92..30f14d609b 100644 --- a/templates/repo/settings/webhook/gitea.tmpl +++ b/templates/repo/settings/webhook/gitea.tmpl @@ -31,10 +31,11 @@ -
- - -
- {{template "repo/settings/webhook/settings" .}} + {{template "repo/settings/webhook/settings" dict + "BaseLink" .BaseLink + "Webhook" .Webhook + "UseAuthorizationHeader" "optional" + "UseRequestSecret" "optional" + }} {{end}} diff --git a/templates/repo/settings/webhook/gogs.tmpl b/templates/repo/settings/webhook/gogs.tmpl index e91a3279e4..c0e054602a 100644 --- a/templates/repo/settings/webhook/gogs.tmpl +++ b/templates/repo/settings/webhook/gogs.tmpl @@ -19,10 +19,11 @@ -
- - -
- {{template "repo/settings/webhook/settings" .}} + {{template "repo/settings/webhook/settings" dict + "BaseLink" .BaseLink + "Webhook" .Webhook + "UseAuthorizationHeader" "optional" + "UseRequestSecret" "optional" + }} {{end}} diff --git a/templates/repo/settings/webhook/list.tmpl b/templates/repo/settings/webhook/list.tmpl deleted file mode 100644 index b24159fccb..0000000000 --- a/templates/repo/settings/webhook/list.tmpl +++ /dev/null @@ -1,4 +0,0 @@ - -{{template "repo/settings/webhook/base_list" .}} - -{{template "repo/settings/webhook/delete_modal" .}} diff --git a/templates/repo/settings/webhook/matrix.tmpl b/templates/repo/settings/webhook/matrix.tmpl index 7f1c9f08e6..e0aad2d807 100644 --- a/templates/repo/settings/webhook/matrix.tmpl +++ b/templates/repo/settings/webhook/matrix.tmpl @@ -22,6 +22,6 @@ - {{template "repo/settings/webhook/settings" .}} + {{template "repo/settings/webhook/settings" dict "BaseLink" .BaseLink "Webhook" .Webhook "UseAuthorizationHeader" "required"}} {{end}} diff --git a/templates/repo/settings/webhook/msteams.tmpl b/templates/repo/settings/webhook/msteams.tmpl index 62ea24e763..17718a1064 100644 --- a/templates/repo/settings/webhook/msteams.tmpl +++ b/templates/repo/settings/webhook/msteams.tmpl @@ -6,6 +6,7 @@ - {{template "repo/settings/webhook/settings" .}} + {{/* FIXME: support authorization header or not? */}} + {{template "repo/settings/webhook/settings" dict "BaseLink" .BaseLink "Webhook" .Webhook "UseAuthorizationHeader" "optional"}} {{end}} diff --git a/templates/repo/settings/webhook/packagist.tmpl b/templates/repo/settings/webhook/packagist.tmpl index 25aba2a435..c813e7c2af 100644 --- a/templates/repo/settings/webhook/packagist.tmpl +++ b/templates/repo/settings/webhook/packagist.tmpl @@ -14,6 +14,7 @@ - {{template "repo/settings/webhook/settings" .}} + {{/* FIXME: support authorization header or not? */}} + {{template "repo/settings/webhook/settings" dict "BaseLink" .BaseLink "Webhook" .Webhook "UseAuthorizationHeader" "optional"}} {{end}} diff --git a/templates/repo/settings/webhook/settings.tmpl b/templates/repo/settings/webhook/settings.tmpl index a330448c9e..a8ad1d6c9e 100644 --- a/templates/repo/settings/webhook/settings.tmpl +++ b/templates/repo/settings/webhook/settings.tmpl @@ -1,4 +1,52 @@ -{{$isNew:=or .PageIsSettingsHooksNew .PageIsAdminDefaultHooksNew .PageIsAdminSystemHooksNew}} +{{/* Template attributes: +- BaseLink: Base URL for the repository settings +- WebHook: Webhook object containing details about the webhook +- UseAuthorizationHeader: optional or required +- UseRequestSecret: optional or required +*/}} +{{$isNew := not .Webhook.ID}} + +
+
+ + + {{ctx.Locale.Tr "repo.settings.active_helper"}} +
+
+ + +{{if .UseAuthorizationHeader}} + {{$attributeValid := or (eq .UseAuthorizationHeader "optional") (eq .UseAuthorizationHeader "required")}} + {{if not $attributeValid}}
Invalid UseAuthorizationHeader: {{.UseAuthorizationHeader}}}
{{end}} + {{$required := eq .UseAuthorizationHeader "required"}} +
+ + + {{if not $required}} + {{ctx.Locale.Tr "repo.settings.authorization_header_desc" (HTMLFormat "%s, %s" "Bearer token123456" "Basic YWxhZGRpbjpvcGVuc2VzYW1l")}} + {{end}} +
+{{end}} + + +{{if .UseRequestSecret}} + {{$attributeValid := or (eq .UseRequestSecret "optional") (eq .UseRequestSecret "required")}} + {{if not $attributeValid}}
Invalid UseRequestSecret: {{.UseRequestSecret}}}
{{end}} + {{$required := eq .UseRequestSecret "required"}} +
+ + + {{ctx.Locale.Tr "repo.settings.webhook_secret_desc"}} +
+{{end}} + + +
+ + + {{ctx.Locale.Tr "repo.settings.branch_filter_desc" "https://pkg.go.dev/github.com/gobwas/glob#Compile" "github.com/gobwas/glob"}} +
+

{{ctx.Locale.Tr "repo.settings.event_desc"}}

@@ -286,38 +334,14 @@
- -
- - - {{ctx.Locale.Tr "repo.settings.branch_filter_desc" "https://pkg.go.dev/github.com/gobwas/glob#Compile" "github.com/gobwas/glob"}} -
- - -
- - - {{if ne .HookType "matrix"}}{{/* Matrix doesn't make the authorization optional but it is implied by the help string, should be changed.*/}} - {{ctx.Locale.Tr "repo.settings.authorization_header_desc" (HTMLFormat "%s, %s" "Bearer token123456" "Basic YWxhZGRpbjpvcGVuc2VzYW1l")}} - {{end}} -
- -
- -
-
- - - {{ctx.Locale.Tr "repo.settings.active_helper"}} -
-
{{if $isNew}} {{else}} - {{ctx.Locale.Tr "repo.settings.delete_webhook"}} + {{ctx.Locale.Tr "repo.settings.delete_webhook"}} {{end}}
- -{{template "repo/settings/webhook/delete_modal" .}} diff --git a/templates/repo/settings/webhook/slack.tmpl b/templates/repo/settings/webhook/slack.tmpl index e7cae92d4b..519d6afa1a 100644 --- a/templates/repo/settings/webhook/slack.tmpl +++ b/templates/repo/settings/webhook/slack.tmpl @@ -23,6 +23,7 @@ - {{template "repo/settings/webhook/settings" .}} + {{/* FIXME: support authorization header or not? */}} + {{template "repo/settings/webhook/settings" dict "BaseLink" .BaseLink "Webhook" .Webhook "UseAuthorizationHeader" "optional"}} {{end}} diff --git a/templates/repo/settings/webhook/telegram.tmpl b/templates/repo/settings/webhook/telegram.tmpl index f92c2be0db..5ab89b72cc 100644 --- a/templates/repo/settings/webhook/telegram.tmpl +++ b/templates/repo/settings/webhook/telegram.tmpl @@ -14,6 +14,7 @@ - {{template "repo/settings/webhook/settings" .}} + {{/* FIXME: support authorization header or not? */}} + {{template "repo/settings/webhook/settings" dict "BaseLink" .BaseLink "Webhook" .Webhook "UseAuthorizationHeader" "optional"}} {{end}} diff --git a/templates/repo/settings/webhook/wechatwork.tmpl b/templates/repo/settings/webhook/wechatwork.tmpl index 78a1617123..cbc29b4610 100644 --- a/templates/repo/settings/webhook/wechatwork.tmpl +++ b/templates/repo/settings/webhook/wechatwork.tmpl @@ -6,6 +6,7 @@ - {{template "repo/settings/webhook/settings" .}} + {{/* FIXME: support authorization header or not? */}} + {{template "repo/settings/webhook/settings" dict "BaseLink" .BaseLink "Webhook" .Webhook "UseAuthorizationHeader" "optional"}} {{end}} diff --git a/templates/user/settings/hooks.tmpl b/templates/user/settings/hooks.tmpl index 477c333220..e2d18001f2 100644 --- a/templates/user/settings/hooks.tmpl +++ b/templates/user/settings/hooks.tmpl @@ -1,5 +1,5 @@ {{template "user/settings/layout_head" (dict "ctxData" . "pageClass" "user settings webhooks")}}
- {{template "repo/settings/webhook/list" .}} + {{template "repo/settings/webhook/base_list" .}}
{{template "user/settings/layout_footer" .}} From 9dafcc5c9e779fba1957400a76041ed091660710 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 6 Jul 2025 23:37:26 +0800 Subject: [PATCH 59/62] Improve project & label color picker and image scroll (#34971) Fix #34609 Fix #34967 --- templates/projects/view.tmpl | 2 +- templates/repo/issue/label_precolors.tmpl | 43 +++++++++++-------- .../repo/issue/labels/label_edit_modal.tmpl | 2 +- web_src/css/base.css | 13 ------ web_src/css/features/colorpicker.css | 32 +++++++++++--- web_src/css/features/projects.css | 3 +- web_src/js/features/colorpicker.ts | 36 ++++++++++------ web_src/js/features/comp/LabelEdit.ts | 2 +- 8 files changed, 76 insertions(+), 57 deletions(-) diff --git a/templates/projects/view.tmpl b/templates/projects/view.tmpl index 6aa776da02..692808a32d 100644 --- a/templates/projects/view.tmpl +++ b/templates/projects/view.tmpl @@ -134,7 +134,7 @@
-
+
{{template "repo/issue/label_precolors"}}
diff --git a/templates/repo/issue/label_precolors.tmpl b/templates/repo/issue/label_precolors.tmpl index 80007662c0..7be3f40350 100644 --- a/templates/repo/issue/label_precolors.tmpl +++ b/templates/repo/issue/label_precolors.tmpl @@ -1,22 +1,27 @@
-
- - - - - - - - -
-
- - - - - - - - + +
+
+ + + + + + + + +
+
+ + + + + + + + +
diff --git a/templates/repo/issue/labels/label_edit_modal.tmpl b/templates/repo/issue/labels/label_edit_modal.tmpl index 6837d66dce..ec57de2f3f 100644 --- a/templates/repo/issue/labels/label_edit_modal.tmpl +++ b/templates/repo/issue/labels/label_edit_modal.tmpl @@ -49,7 +49,7 @@
-
+
{{template "repo/issue/label_precolors"}} diff --git a/web_src/css/base.css b/web_src/css/base.css index 529ddd5386..2b7a47edf1 100644 --- a/web_src/css/base.css +++ b/web_src/css/base.css @@ -1031,19 +1031,6 @@ table th[data-sortt-desc] .svg { min-height: 0; } -.precolors { - display: flex; - flex-direction: column; - justify-content: center; - margin-left: 1em; -} - -.precolors .color { - display: inline-block; - width: 15px; - height: 15px; -} - .ui.dropdown:not(.button) { line-height: var(--line-height-default); /* the dropdown doesn't have default line-height, use this to make the dropdown icon align with plain dropdown */ } diff --git a/web_src/css/features/colorpicker.css b/web_src/css/features/colorpicker.css index b7436783df..4c517e6348 100644 --- a/web_src/css/features/colorpicker.css +++ b/web_src/css/features/colorpicker.css @@ -1,15 +1,13 @@ -.js-color-picker-input { +.color-picker-combo { display: flex; - position: relative; + position: relative; /* to position the preview square */ } -.js-color-picker-input input { - padding-top: 8px !important; - padding-bottom: 8px !important; +.color-picker-combo input { padding-left: 32px !important; } -.js-color-picker-input .preview-square { +.color-picker-combo .preview-square { position: absolute; aspect-ratio: 1; height: 16px; @@ -22,7 +20,7 @@ background-size: 8px 8px; } -.js-color-picker-input .preview-square::after { +.color-picker-combo .preview-square::after { content: ""; position: absolute; width: 100%; @@ -31,6 +29,26 @@ background-color: currentcolor; } +.color-picker-combo .precolors { + display: flex; + margin-left: 1em; + align-items: center; + gap: 0.125em; +} + +.color-picker-combo .precolors .generate-random-color { + padding: 0; + width: 30px; + height: 30px; + min-height: 0; +} + +.color-picker-combo .precolors .color { + display: inline-block; + width: 15px; + height: 15px; +} + hex-color-picker { width: 180px; height: 120px; diff --git a/web_src/css/features/projects.css b/web_src/css/features/projects.css index 7fd5150970..25cb530f85 100644 --- a/web_src/css/features/projects.css +++ b/web_src/css/features/projects.css @@ -71,7 +71,7 @@ .card-attachment-images { display: inline-block; white-space: nowrap; - overflow: scroll; + overflow: auto; cursor: default; scroll-snap-type: x mandatory; text-align: center; @@ -85,6 +85,7 @@ scroll-snap-align: center; margin-right: 2px; aspect-ratio: 1; + object-fit: contain; } .card-attachment-images img:only-child { diff --git a/web_src/js/features/colorpicker.ts b/web_src/js/features/colorpicker.ts index b99e2f8c45..66d1fcb72a 100644 --- a/web_src/js/features/colorpicker.ts +++ b/web_src/js/features/colorpicker.ts @@ -1,18 +1,19 @@ import {createTippy} from '../modules/tippy.ts'; import type {DOMEvent} from '../utils/dom.ts'; +import {registerGlobalInitFunc} from '../modules/observer.ts'; export async function initColorPickers() { - const els = document.querySelectorAll('.js-color-picker-input'); - if (!els.length) return; - - await Promise.all([ - import(/* webpackChunkName: "colorpicker" */'vanilla-colorful/hex-color-picker.js'), - import(/* webpackChunkName: "colorpicker" */'../../css/features/colorpicker.css'), - ]); - - for (const el of els) { + let imported = false; + registerGlobalInitFunc('initColorPicker', async (el) => { + if (!imported) { + await Promise.all([ + import(/* webpackChunkName: "colorpicker" */'vanilla-colorful/hex-color-picker.js'), + import(/* webpackChunkName: "colorpicker" */'../../css/features/colorpicker.css'), + ]); + imported = true; + } initPicker(el); - } + }); } function updateSquare(el: HTMLElement, newValue: string): void { @@ -55,13 +56,20 @@ function initPicker(el: HTMLElement): void { }, }); - // init precolors + // init random color & precolors + const setSelectedColor = (color: string) => { + input.value = color; + input.dispatchEvent(new Event('input', {bubbles: true})); + updateSquare(square, color); + }; + el.querySelector('.generate-random-color').addEventListener('click', () => { + const newValue = `#${Math.floor(Math.random() * 0xFFFFFF).toString(16).padStart(6, '0')}`; + setSelectedColor(newValue); + }); for (const colorEl of el.querySelectorAll('.precolors .color')) { colorEl.addEventListener('click', (e: DOMEvent) => { const newValue = e.target.getAttribute('data-color-hex'); - input.value = newValue; - input.dispatchEvent(new Event('input', {bubbles: true})); - updateSquare(square, newValue); + setSelectedColor(newValue); }); } } diff --git a/web_src/js/features/comp/LabelEdit.ts b/web_src/js/features/comp/LabelEdit.ts index 423440129c..3e27eac1c5 100644 --- a/web_src/js/features/comp/LabelEdit.ts +++ b/web_src/js/features/comp/LabelEdit.ts @@ -24,7 +24,7 @@ export function initCompLabelEdit(pageSelector: string) { const elIsArchivedField = elModal.querySelector('.label-is-archived-input-field'); const elIsArchivedInput = elModal.querySelector('.label-is-archived-input'); const elDescInput = elModal.querySelector('.label-desc-input'); - const elColorInput = elModal.querySelector('.js-color-picker-input input'); + const elColorInput = elModal.querySelector('.color-picker-combo input'); const syncModalUi = () => { const hasScope = nameHasScope(elNameInput.value); From ba943fb77322d7d575b0f06cae2b2f66f5b5d65d Mon Sep 17 00:00:00 2001 From: MrMars98 Date: Sun, 6 Jul 2025 18:27:26 +0200 Subject: [PATCH 60/62] Fixed minor typos in two files #HSFDPMUW (#34944) Fixed minor typos in CODE_OF_CONDUCT.md and README.md I use the hashtag for a project at my university --------- Signed-off-by: MrMars98 --- CODE_OF_CONDUCT.md | 4 ++-- README.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md index 979831eb9b..6a7126388e 100644 --- a/CODE_OF_CONDUCT.md +++ b/CODE_OF_CONDUCT.md @@ -30,7 +30,7 @@ These are the values to which people in the Gitea community should aspire. - **Be constructive.** - Avoid derailing: stay on topic; if you want to talk about something else, start a new conversation. - Avoid unconstructive criticism: don't merely decry the current state of affairs; offer—or at least solicit—suggestions as to how things may be improved. - - Avoid snarking (pithy, unproductive, sniping comments) + - Avoid snarking (pithy, unproductive, sniping comments). - Avoid discussing potentially offensive or sensitive issues; this all too often leads to unnecessary conflict. - Avoid microaggressions (brief and commonplace verbal, behavioral and environmental indignities that communicate hostile, derogatory or negative slights and insults to a person or group). - **Be responsible.** @@ -42,7 +42,7 @@ People are complicated. You should expect to be misunderstood and to misundersta ### Our Pledge -In the interest of fostering an open and welcoming environment, we as contributors and maintainers pledge to making participation in our project and our community a harassment-free experience for everyone, regardless of age, body size, disability, ethnicity, gender identity and expression, level of experience, education, socio-economic status, nationality, personal appearance, race, religion, or sexual identity and orientation. +In the interest of fostering an open and welcoming environment, we as contributors and maintainers pledge to make participation in our project and our community a harassment-free experience for everyone, regardless of age, body size, disability, ethnicity, gender identity and expression, level of experience, education, socio-economic status, nationality, personal appearance, race, religion, or sexual identity and orientation. ### Our Standards diff --git a/README.md b/README.md index 017ca629d0..aa2c6010fa 100644 --- a/README.md +++ b/README.md @@ -80,9 +80,9 @@ Expected workflow is: Fork -> Patch -> Push -> Pull Request [![Crowdin](https://badges.crowdin.net/gitea/localized.svg)](https://translate.gitea.com) -Translations are done through [Crowdin](https://translate.gitea.com). If you want to translate to a new language ask one of the managers in the Crowdin project to add a new language there. +Translations are done through [Crowdin](https://translate.gitea.com). If you want to translate to a new language, ask one of the managers in the Crowdin project to add a new language there. -You can also just create an issue for adding a language or ask on discord on the #translation channel. If you need context or find some translation issues, you can leave a comment on the string or ask on Discord. For general translation questions there is a section in the docs. Currently a bit empty but we hope to fill it as questions pop up. +You can also just create an issue for adding a language or ask on Discord on the #translation channel. If you need context or find some translation issues, you can leave a comment on the string or ask on Discord. For general translation questions there is a section in the docs. Currently a bit empty, but we hope to fill it as questions pop up. Get more information from [documentation](https://docs.gitea.com/contributing/localization). From 95a935aca05af3ccf64ecc2a6c19282dd310702f Mon Sep 17 00:00:00 2001 From: silverwind Date: Sun, 6 Jul 2025 18:53:34 +0200 Subject: [PATCH 61/62] Enable gocritic `equalFold` and fix issues (#34952) Continuation of https://github.com/go-gitea/gitea/pull/34678. --------- Signed-off-by: silverwind --- .golangci.yml | 2 ++ models/repo/language_stats.go | 9 ++++----- modules/markup/mdstripper/mdstripper.go | 3 +-- modules/packages/pub/metadata.go | 2 +- modules/setting/actions.go | 4 ++-- modules/util/slice.go | 3 +-- modules/util/time_str.go | 2 +- routers/api/v1/api.go | 2 +- routers/api/v1/repo/collaborators.go | 2 +- routers/api/v1/repo/repo.go | 2 +- routers/web/repo/setting/setting.go | 2 +- services/auth/source/ldap/source_search.go | 2 +- services/context/org.go | 2 +- services/context/repo.go | 2 +- services/context/user.go | 2 +- services/repository/files/file.go | 2 +- 16 files changed, 21 insertions(+), 22 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 70efd288ff..2ad39fbae2 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -50,6 +50,8 @@ linters: require-explanation: true require-specific: true gocritic: + enabled-checks: + - equalFold disabled-checks: - ifElseChain - singleCaseSwitch # Every time this occurred in the code, there was no other way. diff --git a/models/repo/language_stats.go b/models/repo/language_stats.go index 0bc0f1fb40..7db8cd4dd2 100644 --- a/models/repo/language_stats.go +++ b/models/repo/language_stats.go @@ -157,18 +157,17 @@ func UpdateLanguageStats(ctx context.Context, repo *Repository, commitID string, for lang, size := range stats { if size > s { s = size - topLang = strings.ToLower(lang) + topLang = lang } } for lang, size := range stats { upd := false - llang := strings.ToLower(lang) for _, s := range oldstats { // Update already existing language - if strings.ToLower(s.Language) == llang { + if strings.EqualFold(s.Language, lang) { s.CommitID = commitID - s.IsPrimary = llang == topLang + s.IsPrimary = lang == topLang s.Size = size if _, err := sess.ID(s.ID).Cols("`commit_id`", "`size`", "`is_primary`").Update(s); err != nil { return err @@ -182,7 +181,7 @@ func UpdateLanguageStats(ctx context.Context, repo *Repository, commitID string, if err := db.Insert(ctx, &LanguageStat{ RepoID: repo.ID, CommitID: commitID, - IsPrimary: llang == topLang, + IsPrimary: lang == topLang, Language: lang, Size: size, }); err != nil { diff --git a/modules/markup/mdstripper/mdstripper.go b/modules/markup/mdstripper/mdstripper.go index 6e392444b4..5a6504416a 100644 --- a/modules/markup/mdstripper/mdstripper.go +++ b/modules/markup/mdstripper/mdstripper.go @@ -91,8 +91,7 @@ func (r *stripRenderer) processAutoLink(w io.Writer, link []byte) { } // Note: we're not attempting to match the URL scheme (http/https) - host := strings.ToLower(u.Host) - if host != "" && host != strings.ToLower(r.localhost.Host) { + if u.Host != "" && !strings.EqualFold(u.Host, r.localhost.Host) { // Process out of band r.links = append(r.links, linkStr) return diff --git a/modules/packages/pub/metadata.go b/modules/packages/pub/metadata.go index afb464e462..9b00472eb2 100644 --- a/modules/packages/pub/metadata.go +++ b/modules/packages/pub/metadata.go @@ -88,7 +88,7 @@ func ParsePackage(r io.Reader) (*Package, error) { if err != nil { return nil, err } - } else if strings.ToLower(hd.Name) == "readme.md" { + } else if strings.EqualFold(hd.Name, "readme.md") { data, err := io.ReadAll(tr) if err != nil { return nil, err diff --git a/modules/setting/actions.go b/modules/setting/actions.go index 913872eaf2..8bace1f750 100644 --- a/modules/setting/actions.go +++ b/modules/setting/actions.go @@ -62,11 +62,11 @@ func (c logCompression) IsValid() bool { } func (c logCompression) IsNone() bool { - return strings.ToLower(string(c)) == "none" + return string(c) == "none" } func (c logCompression) IsZstd() bool { - return c == "" || strings.ToLower(string(c)) == "zstd" + return c == "" || string(c) == "zstd" } func loadActionsFrom(rootCfg ConfigProvider) error { diff --git a/modules/util/slice.go b/modules/util/slice.go index da6886491e..aaa729c1c9 100644 --- a/modules/util/slice.go +++ b/modules/util/slice.go @@ -12,8 +12,7 @@ import ( // SliceContainsString sequential searches if string exists in slice. func SliceContainsString(slice []string, target string, insensitive ...bool) bool { if len(insensitive) != 0 && insensitive[0] { - target = strings.ToLower(target) - return slices.ContainsFunc(slice, func(t string) bool { return strings.ToLower(t) == target }) + return slices.ContainsFunc(slice, func(t string) bool { return strings.EqualFold(t, target) }) } return slices.Contains(slice, target) diff --git a/modules/util/time_str.go b/modules/util/time_str.go index 0fccfe82cc..81b132c3db 100644 --- a/modules/util/time_str.go +++ b/modules/util/time_str.go @@ -59,7 +59,7 @@ func TimeEstimateParse(timeStr string) (int64, error) { unit := timeStr[match[4]:match[5]] found := false for _, u := range timeStrGlobalVars().units { - if strings.ToLower(unit) == u.name { + if strings.EqualFold(unit, u.name) { total += amount * u.num found = true break diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 4a4bf12657..f412e8a06c 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -145,7 +145,7 @@ func repoAssignment() func(ctx *context.APIContext) { ) // Check if the user is the same as the repository owner. - if ctx.IsSigned && ctx.Doer.LowerName == strings.ToLower(userName) { + if ctx.IsSigned && strings.EqualFold(ctx.Doer.LowerName, userName) { owner = ctx.Doer } else { owner, err = user_model.GetUserByName(ctx, userName) diff --git a/routers/api/v1/repo/collaborators.go b/routers/api/v1/repo/collaborators.go index c2c10cc695..eed9c19fe1 100644 --- a/routers/api/v1/repo/collaborators.go +++ b/routers/api/v1/repo/collaborators.go @@ -276,7 +276,7 @@ func GetRepoPermissions(ctx *context.APIContext) { // "$ref": "#/responses/forbidden" collaboratorUsername := ctx.PathParam("collaborator") - if !ctx.Doer.IsAdmin && ctx.Doer.LowerName != strings.ToLower(collaboratorUsername) && !ctx.IsUserRepoAdmin() { + if !ctx.Doer.IsAdmin && !strings.EqualFold(ctx.Doer.LowerName, collaboratorUsername) && !ctx.IsUserRepoAdmin() { ctx.APIError(http.StatusForbidden, "Only admins can query all permissions, repo admins can query all repo permissions, collaborators can query only their own") return } diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 8acc912796..292b267c01 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -669,7 +669,7 @@ func updateBasicProperties(ctx *context.APIContext, opts api.EditRepoOption) err newRepoName = *opts.Name } // Check if repository name has been changed and not just a case change - if repo.LowerName != strings.ToLower(newRepoName) { + if !strings.EqualFold(repo.LowerName, newRepoName) { if err := repo_service.ChangeRepositoryName(ctx, ctx.Doer, repo, newRepoName); err != nil { switch { case repo_model.IsErrRepoAlreadyExist(err): diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index 6e16ead183..0865d9d7c0 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -165,7 +165,7 @@ func handleSettingsPostUpdate(ctx *context.Context) { newRepoName := form.RepoName // Check if repository name has been changed. - if repo.LowerName != strings.ToLower(newRepoName) { + if !strings.EqualFold(repo.LowerName, newRepoName) { // Close the GitRepo if open if ctx.Repo.GitRepo != nil { ctx.Repo.GitRepo.Close() diff --git a/services/auth/source/ldap/source_search.go b/services/auth/source/ldap/source_search.go index e6bce04a83..f6c032492f 100644 --- a/services/auth/source/ldap/source_search.go +++ b/services/auth/source/ldap/source_search.go @@ -241,7 +241,7 @@ func (source *Source) listLdapGroupMemberships(l *ldap.Conn, uid string, applyGr } func (source *Source) getUserAttributeListedInGroup(entry *ldap.Entry) string { - if strings.ToLower(source.UserUID) == "dn" { + if strings.EqualFold(source.UserUID, "dn") { return entry.DN } diff --git a/services/context/org.go b/services/context/org.go index c8b6ed09b7..1cd8923178 100644 --- a/services/context/org.go +++ b/services/context/org.go @@ -208,7 +208,7 @@ func OrgAssignment(opts OrgAssignmentOptions) func(ctx *Context) { if len(teamName) > 0 { teamExists := false for _, team := range ctx.Org.Teams { - if team.LowerName == strings.ToLower(teamName) { + if strings.EqualFold(team.LowerName, teamName) { teamExists = true ctx.Org.Team = team ctx.Org.IsTeamMember = true diff --git a/services/context/repo.go b/services/context/repo.go index 572211712b..afc6de9b16 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -429,7 +429,7 @@ func RepoAssignment(ctx *Context) { } // Check if the user is the same as the repository owner - if ctx.IsSigned && ctx.Doer.LowerName == strings.ToLower(userName) { + if ctx.IsSigned && strings.EqualFold(ctx.Doer.LowerName, userName) { ctx.Repo.Owner = ctx.Doer } else { ctx.Repo.Owner, err = user_model.GetUserByName(ctx, userName) diff --git a/services/context/user.go b/services/context/user.go index c09ded8339..f1a3035ee9 100644 --- a/services/context/user.go +++ b/services/context/user.go @@ -61,7 +61,7 @@ func UserAssignmentAPI() func(ctx *APIContext) { func userAssignment(ctx *Base, doer *user_model.User, errCb func(int, any)) (contextUser *user_model.User) { username := ctx.PathParam("username") - if doer != nil && doer.LowerName == strings.ToLower(username) { + if doer != nil && strings.EqualFold(doer.LowerName, username) { contextUser = doer } else { var err error diff --git a/services/repository/files/file.go b/services/repository/files/file.go index 13d171d139..f48e32b427 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -144,7 +144,7 @@ func CleanGitTreePath(name string) string { name = util.PathJoinRel(name) // Git disallows any filenames to have a .git directory in them. for part := range strings.SplitSeq(name, "/") { - if strings.ToLower(part) == ".git" { + if strings.EqualFold(part, ".git") { return "" } } From 6b42ea1e54531650e99503aa39158ec8593c0c98 Mon Sep 17 00:00:00 2001 From: NorthRealm <155140859+NorthRealm@users.noreply.github.com> Date: Mon, 7 Jul 2025 01:47:02 +0800 Subject: [PATCH 62/62] Rerun job only when run is done (#34970) For consistency, limit rerunning Job(s) to only when Run is in Done status. --- routers/web/repo/actions/view.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index 7e1b923fa4..52b2e9995e 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -249,7 +249,7 @@ func ViewPost(ctx *context_module.Context) { ID: v.ID, Name: v.Name, Status: v.Status.String(), - CanRerun: v.Status.IsDone() && ctx.Repo.CanWrite(unit.TypeActions), + CanRerun: resp.State.Run.CanRerun, Duration: v.Duration().String(), }) } @@ -445,7 +445,7 @@ func Rerun(ctx *context_module.Context) { return } } - ctx.JSON(http.StatusOK, struct{}{}) + ctx.JSONOK() return } @@ -460,12 +460,12 @@ func Rerun(ctx *context_module.Context) { } } - ctx.JSON(http.StatusOK, struct{}{}) + ctx.JSONOK() } func rerunJob(ctx *context_module.Context, job *actions_model.ActionRunJob, shouldBlock bool) error { status := job.Status - if !status.IsDone() { + if !status.IsDone() || !job.Run.Status.IsDone() { return nil }