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 <me@silverwind.io>
go-gitea · Jun 18, 2024 · 37a4b23 · 37a4b23
1 parent d32648b
commit 37a4b23
Show file tree

Hide file tree

Showing 11 changed files with 25 additions and 55 deletions.
diff --git 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
@@ -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
@@ -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("<UnitType:%d:%s>", u, u.String())
+	unit, ok := Units[u]
+	unitName := "unknown"
+	if ok {
+		unitName = unit.NameKey
+	}
+	return fmt.Sprintf("<UnitType:%d:%s>", 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
@@ -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
@@ -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
@@ -35,13 +35,13 @@
 		{{if and .IsSigned .MustChangePassword}}
 			{{/* No links */}}
 		{{else if .IsSigned}}
-			{{if not .UnitIssuesGlobalDisabled}}
+			{{if not ctx.Consts.RepoUnitTypeIssues.UnitGlobalDisabled}}
 				<a class="item{{if .PageIsIssues}} active{{end}}" href="{{AppSubUrl}}/issues">{{ctx.Locale.Tr "issues"}}</a>
 			{{end}}
-			{{if not .UnitPullsGlobalDisabled}}
+			{{if not ctx.Consts.RepoUnitTypePullRequests.UnitGlobalDisabled}}
 				<a class="item{{if .PageIsPulls}} active{{end}}" href="{{AppSubUrl}}/pulls">{{ctx.Locale.Tr "pull_requests"}}</a>
 			{{end}}
-			{{if not (and .UnitIssuesGlobalDisabled .UnitPullsGlobalDisabled)}}
+			{{if not (and ctx.Consts.RepoUnitTypeIssues.UnitGlobalDisabled ctx.Consts.RepoUnitTypePullRequests.UnitGlobalDisabled)}}
 				{{if .ShowMilestonesDashboardPage}}
 					<a class="item{{if .PageIsMilestonesDashboard}} active{{end}}" href="{{AppSubUrl}}/milestones">{{ctx.Locale.Tr "milestones"}}</a>
 				{{end}}

diff --git a/templates/repo/header.tmpl b/templates/repo/header.tmpl
@@ -162,7 +162,7 @@
 						</a>
 					{{end}}
 
-					{{if and .EnableActions (not .UnitActionsGlobalDisabled) (.Permission.CanRead ctx.Consts.RepoUnitTypeActions)}}
+					{{if and .EnableActions (.Permission.CanRead ctx.Consts.RepoUnitTypeActions)}}
 						<a class="{{if .PageIsActions}}active {{end}}item" href="{{.RepoLink}}/actions">
 							{{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")}}
 						<a href="{{.RepoLink}}/projects" class="{{if .IsProjectsPage}}active {{end}}item">
 							{{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
@@ -574,7 +574,6 @@
 				{{template "repo/commits_list_small" dict "comment" . "root" $}}
 			{{end}}
 		{{else if eq .Type 30}}
-			{{if not $.UnitProjectsGlobalDisabled}}
 			<div class="timeline-item event" id="{{.HashTag}}">
 				<span class="badge">{{svg "octicon-project"}}</span>
 				{{template "shared/user/avatarlink" dict "user" .Poster}}
@@ -599,7 +598,6 @@
 					{{end}}
 				</span>
 			</div>
-			{{end}}
 		{{else if eq .Type 32}}
 			<div class="timeline-item-group">
 				<div class="timeline-item event" id="{{.HashTag}}">

diff --git 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}}
 				<div class="item context js-aria-clickable quote-reply {{if .diff}}quote-reply-diff{{end}}" data-target="{{.item.HashTag}}-raw">{{ctx.Locale.Tr "repo.issues.context.quote_reply"}}</div>
-				{{if not .ctxData.UnitIssuesGlobalDisabled}}
+				{{if not ctx.Consts.RepoUnitTypeIssues.UnitGlobalDisabled}}
 					<div class="item context js-aria-clickable reference-issue" data-target="{{.item.HashTag}}-raw" data-modal="#reference-issue-modal" data-poster="{{.item.Poster.GetDisplayName}}" data-poster-username="{{.item.Poster.Name}}" data-reference="{{$referenceUrl}}">{{ctx.Locale.Tr "repo.issues.context.reference_issue"}}</div>
 				{{end}}
 				{{if or .ctxData.Permission.IsAdmin .IsCommentPoster .ctxData.HasIssuesOrPullsWritePermission}}

diff --git a/templates/repo/settings/navbar.tmpl b/templates/repo/settings/navbar.tmpl
@@ -35,7 +35,7 @@
 				</a>
 			{{end}}
 		{{end}}
-		{{if and .EnableActions (not .UnitActionsGlobalDisabled) (.Permission.CanRead ctx.Consts.RepoUnitTypeActions)}}
+		{{if and .EnableActions (.Permission.CanRead ctx.Consts.RepoUnitTypeActions)}}
 		<details class="item toggleable-item" {{if or .PageIsSharedSettingsRunners .PageIsSharedSettingsSecrets .PageIsSharedSettingsVariables}}open{{end}}>
 			<summary>{{ctx.Locale.Tr "actions.actions"}}</summary>
 			<div class="menu">

diff --git a/templates/user/dashboard/navbar.tmpl b/templates/user/dashboard/navbar.tmpl
@@ -81,17 +81,17 @@
 			<a class="{{if .PageIsNews}}active {{end}}item tw-ml-auto" href="{{.ContextUser.DashboardLink}}{{if .Team}}/{{PathEscape .Team.Name}}{{end}}">
 				{{svg "octicon-rss"}}&nbsp;{{ctx.Locale.Tr "activities"}}
 			</a>
-			{{if not .UnitIssuesGlobalDisabled}}
+			{{if not ctx.Consts.RepoUnitTypeIssues.UnitGlobalDisabled}}
 			<a class="{{if .PageIsIssues}}active {{end}}item" href="{{.ContextUser.OrganisationLink}}/issues{{if .Team}}/{{PathEscape .Team.Name}}{{end}}">
 				{{svg "octicon-issue-opened"}}&nbsp;{{ctx.Locale.Tr "issues"}}
 			</a>
 			{{end}}
-			{{if not .UnitPullsGlobalDisabled}}
+			{{if not ctx.Consts.RepoUnitTypePullRequests.UnitGlobalDisabled}}
 			<a class="{{if .PageIsPulls}}active {{end}}item" href="{{.ContextUser.OrganisationLink}}/pulls{{if .Team}}/{{PathEscape .Team.Name}}{{end}}">
 				{{svg "octicon-git-pull-request"}}&nbsp;{{ctx.Locale.Tr "pull_requests"}}
 			</a>
 			{{end}}
-			{{if and .ShowMilestonesDashboardPage (not (and .UnitIssuesGlobalDisabled .UnitPullsGlobalDisabled))}}
+			{{if and .ShowMilestonesDashboardPage (not (and ctx.Consts.RepoUnitTypeIssues.UnitGlobalDisabled ctx.Consts.RepoUnitTypePullRequests.UnitGlobalDisabled))}}
 			<a class="{{if .PageIsMilestonesDashboard}}active {{end}}item" href="{{.ContextUser.OrganisationLink}}/milestones{{if .Team}}/{{PathEscape .Team.Name}}{{end}}">
 				{{svg "octicon-milestone"}}&nbsp;{{ctx.Locale.Tr "milestones"}}
 			</a>