From 37a4b233a0a4ca516b90e0c8e15d8dafb8d13358 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 18 Jun 2024 08:51:13 +0800 Subject: [PATCH] Refactor repo unit "disabled" check (#31389) 1. There are already global "unit consts", no need to use context data, which is fragile 2. Remove the "String()" method from "unit", it would only cause rendering problems in templates --------- Co-authored-by: silverwind --- models/repo/repo.go | 2 +- models/repo/repo_unit.go | 2 +- models/unit/unit.go | 37 ++++--------------- routers/web/web.go | 8 ++-- services/context/context.go | 9 +---- templates/base/head_navbar.tmpl | 6 +-- templates/repo/header.tmpl | 4 +- .../repo/issue/view_content/comments.tmpl | 2 - .../repo/issue/view_content/context_menu.tmpl | 2 +- templates/repo/settings/navbar.tmpl | 2 +- templates/user/dashboard/navbar.tmpl | 6 +-- 11 files changed, 25 insertions(+), 55 deletions(-) diff --git a/models/repo/repo.go b/models/repo/repo.go index f02c55fc8..189c4aba6 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -362,7 +362,7 @@ func (repo *Repository) LoadUnits(ctx context.Context) (err error) { if log.IsTrace() { unitTypeStrings := make([]string, len(repo.Units)) for i, unit := range repo.Units { - unitTypeStrings[i] = unit.Type.String() + unitTypeStrings[i] = unit.Type.LogString() } log.Trace("repo.Units, ID=%d, Types: [%s]", repo.ID, strings.Join(unitTypeStrings, ", ")) } diff --git a/models/repo/repo_unit.go b/models/repo/repo_unit.go index fd5baa948..cb52c2c9e 100644 --- a/models/repo/repo_unit.go +++ b/models/repo/repo_unit.go @@ -33,7 +33,7 @@ func IsErrUnitTypeNotExist(err error) bool { } func (err ErrUnitTypeNotExist) Error() string { - return fmt.Sprintf("Unit type does not exist: %s", err.UT.String()) + return fmt.Sprintf("Unit type does not exist: %s", err.UT.LogString()) } func (err ErrUnitTypeNotExist) Unwrap() error { diff --git a/models/unit/unit.go b/models/unit/unit.go index 8eedcbd34..3b62e5f98 100644 --- a/models/unit/unit.go +++ b/models/unit/unit.go @@ -33,39 +33,18 @@ const ( TypeActions // 10 Actions ) -// Value returns integer value for unit type +// Value returns integer value for unit type (used by template) func (u Type) Value() int { return int(u) } -func (u Type) String() string { - switch u { - case TypeCode: - return "TypeCode" - case TypeIssues: - return "TypeIssues" - case TypePullRequests: - return "TypePullRequests" - case TypeReleases: - return "TypeReleases" - case TypeWiki: - return "TypeWiki" - case TypeExternalWiki: - return "TypeExternalWiki" - case TypeExternalTracker: - return "TypeExternalTracker" - case TypeProjects: - return "TypeProjects" - case TypePackages: - return "TypePackages" - case TypeActions: - return "TypeActions" - } - return fmt.Sprintf("Unknown Type %d", u) -} - func (u Type) LogString() string { - return fmt.Sprintf("", u, u.String()) + unit, ok := Units[u] + unitName := "unknown" + if ok { + unitName = unit.NameKey + } + return fmt.Sprintf("", u, unitName) } var ( @@ -133,7 +112,7 @@ func validateDefaultRepoUnits(defaultUnits, settingDefaultUnits []Type) []Type { units = make([]Type, 0, len(settingDefaultUnits)) for _, settingUnit := range settingDefaultUnits { if !settingUnit.CanBeDefault() { - log.Warn("Not allowed as default unit: %s", settingUnit.String()) + log.Warn("Not allowed as default unit: %s", settingUnit.LogString()) continue } units = append(units, settingUnit) diff --git a/routers/web/web.go b/routers/web/web.go index 08f5d3d06..715b5d151 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -384,18 +384,18 @@ func registerRoutes(m *web.Route) { return func(ctx *context.Context) { // only check global disabled units when ignoreGlobal is false if !ignoreGlobal && unitType.UnitGlobalDisabled() { - ctx.NotFound(unitType.String(), nil) + ctx.NotFound("Repo unit is is disabled: "+unitType.LogString(), nil) return } if ctx.ContextUser == nil { - ctx.NotFound(unitType.String(), nil) + ctx.NotFound("ContextUser is nil", nil) return } if ctx.ContextUser.IsOrganization() { if ctx.Org.Organization.UnitPermission(ctx, ctx.Doer, unitType) < accessMode { - ctx.NotFound(unitType.String(), nil) + ctx.NotFound("ContextUser is org but doer has no access to unit", nil) return } } @@ -487,7 +487,7 @@ func registerRoutes(m *web.Route) { m.Get("/organizations", explore.Organizations) m.Get("/code", func(ctx *context.Context) { if unit.TypeCode.UnitGlobalDisabled() { - ctx.NotFound(unit.TypeCode.String(), nil) + ctx.NotFound("Repo unit code is disabled", nil) return } }, explore.Code) diff --git a/services/context/context.go b/services/context/context.go index aab0485f1..69b65cbdd 100644 --- a/services/context/context.go +++ b/services/context/context.go @@ -210,16 +210,9 @@ func Contexter() func(next http.Handler) http.Handler { // FIXME: do we really always need these setting? There should be someway to have to avoid having to always set these ctx.Data["DisableMigrations"] = setting.Repository.DisableMigrations ctx.Data["DisableStars"] = setting.Repository.DisableStars - ctx.Data["EnableActions"] = setting.Actions.Enabled + ctx.Data["EnableActions"] = setting.Actions.Enabled && !unit.TypeActions.UnitGlobalDisabled() ctx.Data["ManifestData"] = setting.ManifestData - - ctx.Data["UnitWikiGlobalDisabled"] = unit.TypeWiki.UnitGlobalDisabled() - ctx.Data["UnitIssuesGlobalDisabled"] = unit.TypeIssues.UnitGlobalDisabled() - ctx.Data["UnitPullsGlobalDisabled"] = unit.TypePullRequests.UnitGlobalDisabled() - ctx.Data["UnitProjectsGlobalDisabled"] = unit.TypeProjects.UnitGlobalDisabled() - ctx.Data["UnitActionsGlobalDisabled"] = unit.TypeActions.UnitGlobalDisabled() - ctx.Data["AllLangs"] = translation.AllLangs() next.ServeHTTP(ctx.Resp, ctx.Req) diff --git a/templates/base/head_navbar.tmpl b/templates/base/head_navbar.tmpl index 488992481..7be2d96d7 100644 --- a/templates/base/head_navbar.tmpl +++ b/templates/base/head_navbar.tmpl @@ -35,13 +35,13 @@ {{if and .IsSigned .MustChangePassword}} {{/* No links */}} {{else if .IsSigned}} - {{if not .UnitIssuesGlobalDisabled}} + {{if not ctx.Consts.RepoUnitTypeIssues.UnitGlobalDisabled}} {{ctx.Locale.Tr "issues"}} {{end}} - {{if not .UnitPullsGlobalDisabled}} + {{if not ctx.Consts.RepoUnitTypePullRequests.UnitGlobalDisabled}} {{ctx.Locale.Tr "pull_requests"}} {{end}} - {{if not (and .UnitIssuesGlobalDisabled .UnitPullsGlobalDisabled)}} + {{if not (and ctx.Consts.RepoUnitTypeIssues.UnitGlobalDisabled ctx.Consts.RepoUnitTypePullRequests.UnitGlobalDisabled)}} {{if .ShowMilestonesDashboardPage}} {{ctx.Locale.Tr "milestones"}} {{end}} diff --git a/templates/repo/header.tmpl b/templates/repo/header.tmpl index 22daaab4b..d52891b02 100644 --- a/templates/repo/header.tmpl +++ b/templates/repo/header.tmpl @@ -162,7 +162,7 @@ {{end}} - {{if and .EnableActions (not .UnitActionsGlobalDisabled) (.Permission.CanRead ctx.Consts.RepoUnitTypeActions)}} + {{if and .EnableActions (.Permission.CanRead ctx.Consts.RepoUnitTypeActions)}} {{svg "octicon-play"}} {{ctx.Locale.Tr "actions.actions"}} {{if .Repository.NumOpenActionRuns}} @@ -178,7 +178,7 @@ {{end}} {{$projectsUnit := .Repository.MustGetUnit $.Context ctx.Consts.RepoUnitTypeProjects}} - {{if and (not .UnitProjectsGlobalDisabled) (.Permission.CanRead ctx.Consts.RepoUnitTypeProjects) ($projectsUnit.ProjectsConfig.IsProjectsAllowed "repo")}} + {{if and (not ctx.Consts.RepoUnitTypeProjects.UnitGlobalDisabled) (.Permission.CanRead ctx.Consts.RepoUnitTypeProjects) ($projectsUnit.ProjectsConfig.IsProjectsAllowed "repo")}} {{svg "octicon-project"}} {{ctx.Locale.Tr "repo.projects"}} {{if .Repository.NumOpenProjects}} diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 3da2f3815..d8ca9de7b 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -574,7 +574,6 @@ {{template "repo/commits_list_small" dict "comment" . "root" $}} {{end}} {{else if eq .Type 30}} - {{if not $.UnitProjectsGlobalDisabled}}
{{svg "octicon-project"}} {{template "shared/user/avatarlink" dict "user" .Poster}} @@ -599,7 +598,6 @@ {{end}}
- {{end}} {{else if eq .Type 32}}
diff --git a/templates/repo/issue/view_content/context_menu.tmpl b/templates/repo/issue/view_content/context_menu.tmpl index 17556d4e4..9e38442c3 100644 --- a/templates/repo/issue/view_content/context_menu.tmpl +++ b/templates/repo/issue/view_content/context_menu.tmpl @@ -15,7 +15,7 @@ {{if not .ctxData.Repository.IsArchived}} {{$needDivider = true}}
{{ctx.Locale.Tr "repo.issues.context.quote_reply"}}
- {{if not .ctxData.UnitIssuesGlobalDisabled}} + {{if not ctx.Consts.RepoUnitTypeIssues.UnitGlobalDisabled}}
{{ctx.Locale.Tr "repo.issues.context.reference_issue"}}
{{end}} {{if or .ctxData.Permission.IsAdmin .IsCommentPoster .ctxData.HasIssuesOrPullsWritePermission}} diff --git a/templates/repo/settings/navbar.tmpl b/templates/repo/settings/navbar.tmpl index 414effbf2..b9105bb8e 100644 --- a/templates/repo/settings/navbar.tmpl +++ b/templates/repo/settings/navbar.tmpl @@ -35,7 +35,7 @@ {{end}} {{end}} - {{if and .EnableActions (not .UnitActionsGlobalDisabled) (.Permission.CanRead ctx.Consts.RepoUnitTypeActions)}} + {{if and .EnableActions (.Permission.CanRead ctx.Consts.RepoUnitTypeActions)}}
{{ctx.Locale.Tr "actions.actions"}}
diff --git a/templates/user/dashboard/navbar.tmpl b/templates/user/dashboard/navbar.tmpl index 464228289..7982cbd95 100644 --- a/templates/user/dashboard/navbar.tmpl +++ b/templates/user/dashboard/navbar.tmpl @@ -81,17 +81,17 @@ {{svg "octicon-rss"}} {{ctx.Locale.Tr "activities"}} - {{if not .UnitIssuesGlobalDisabled}} + {{if not ctx.Consts.RepoUnitTypeIssues.UnitGlobalDisabled}} {{svg "octicon-issue-opened"}} {{ctx.Locale.Tr "issues"}} {{end}} - {{if not .UnitPullsGlobalDisabled}} + {{if not ctx.Consts.RepoUnitTypePullRequests.UnitGlobalDisabled}} {{svg "octicon-git-pull-request"}} {{ctx.Locale.Tr "pull_requests"}} {{end}} - {{if and .ShowMilestonesDashboardPage (not (and .UnitIssuesGlobalDisabled .UnitPullsGlobalDisabled))}} + {{if and .ShowMilestonesDashboardPage (not (and ctx.Consts.RepoUnitTypeIssues.UnitGlobalDisabled ctx.Consts.RepoUnitTypePullRequests.UnitGlobalDisabled))}} {{svg "octicon-milestone"}} {{ctx.Locale.Tr "milestones"}}