Skip to content

Commit 37a4b23

Browse files
wxiaoguangsilverwind
wxiaoguang
and
silverwind
authored
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>
1 parent d32648b commit 37a4b23

File tree

11 files changed

+25
-55
lines changed

11 files changed

+25
-55
lines changed

‎models/repo/repo.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ func (repo *Repository) LoadUnits(ctx context.Context) (err error) {
362362
if log.IsTrace() {
363363
unitTypeStrings := make([]string, len(repo.Units))
364364
for i, unit := range repo.Units {
365-
unitTypeStrings[i] = unit.Type.String()
365+
unitTypeStrings[i] = unit.Type.LogString()
366366
}
367367
log.Trace("repo.Units, ID=%d, Types: [%s]", repo.ID, strings.Join(unitTypeStrings, ", "))
368368
}

‎models/repo/repo_unit.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func IsErrUnitTypeNotExist(err error) bool {
3333
}
3434

3535
func (err ErrUnitTypeNotExist) Error() string {
36-
return fmt.Sprintf("Unit type does not exist: %s", err.UT.String())
36+
return fmt.Sprintf("Unit type does not exist: %s", err.UT.LogString())
3737
}
3838

3939
func (err ErrUnitTypeNotExist) Unwrap() error {

‎models/unit/unit.go

+8-29
Original file line numberDiff line numberDiff line change
@@ -33,39 +33,18 @@ const (
3333
TypeActions // 10 Actions
3434
)
3535

36-
// Value returns integer value for unit type
36+
// Value returns integer value for unit type (used by template)
3737
func (u Type) Value() int {
3838
return int(u)
3939
}
4040

41-
func (u Type) String() string {
42-
switch u {
43-
case TypeCode:
44-
return "TypeCode"
45-
case TypeIssues:
46-
return "TypeIssues"
47-
case TypePullRequests:
48-
return "TypePullRequests"
49-
case TypeReleases:
50-
return "TypeReleases"
51-
case TypeWiki:
52-
return "TypeWiki"
53-
case TypeExternalWiki:
54-
return "TypeExternalWiki"
55-
case TypeExternalTracker:
56-
return "TypeExternalTracker"
57-
case TypeProjects:
58-
return "TypeProjects"
59-
case TypePackages:
60-
return "TypePackages"
61-
case TypeActions:
62-
return "TypeActions"
63-
}
64-
return fmt.Sprintf("Unknown Type %d", u)
65-
}
66-
6741
func (u Type) LogString() string {
68-
return fmt.Sprintf("<UnitType:%d:%s>", u, u.String())
42+
unit, ok := Units[u]
43+
unitName := "unknown"
44+
if ok {
45+
unitName = unit.NameKey
46+
}
47+
return fmt.Sprintf("<UnitType:%d:%s>", u, unitName)
6948
}
7049

7150
var (
@@ -133,7 +112,7 @@ func validateDefaultRepoUnits(defaultUnits, settingDefaultUnits []Type) []Type {
133112
units = make([]Type, 0, len(settingDefaultUnits))
134113
for _, settingUnit := range settingDefaultUnits {
135114
if !settingUnit.CanBeDefault() {
136-
log.Warn("Not allowed as default unit: %s", settingUnit.String())
115+
log.Warn("Not allowed as default unit: %s", settingUnit.LogString())
137116
continue
138117
}
139118
units = append(units, settingUnit)

‎routers/web/web.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -384,18 +384,18 @@ func registerRoutes(m *web.Route) {
384384
return func(ctx *context.Context) {
385385
// only check global disabled units when ignoreGlobal is false
386386
if !ignoreGlobal && unitType.UnitGlobalDisabled() {
387-
ctx.NotFound(unitType.String(), nil)
387+
ctx.NotFound("Repo unit is is disabled: "+unitType.LogString(), nil)
388388
return
389389
}
390390

391391
if ctx.ContextUser == nil {
392-
ctx.NotFound(unitType.String(), nil)
392+
ctx.NotFound("ContextUser is nil", nil)
393393
return
394394
}
395395

396396
if ctx.ContextUser.IsOrganization() {
397397
if ctx.Org.Organization.UnitPermission(ctx, ctx.Doer, unitType) < accessMode {
398-
ctx.NotFound(unitType.String(), nil)
398+
ctx.NotFound("ContextUser is org but doer has no access to unit", nil)
399399
return
400400
}
401401
}
@@ -487,7 +487,7 @@ func registerRoutes(m *web.Route) {
487487
m.Get("/organizations", explore.Organizations)
488488
m.Get("/code", func(ctx *context.Context) {
489489
if unit.TypeCode.UnitGlobalDisabled() {
490-
ctx.NotFound(unit.TypeCode.String(), nil)
490+
ctx.NotFound("Repo unit code is disabled", nil)
491491
return
492492
}
493493
}, explore.Code)

‎services/context/context.go

+1-8
Original file line numberDiff line numberDiff line change
@@ -210,16 +210,9 @@ func Contexter() func(next http.Handler) http.Handler {
210210
// FIXME: do we really always need these setting? There should be someway to have to avoid having to always set these
211211
ctx.Data["DisableMigrations"] = setting.Repository.DisableMigrations
212212
ctx.Data["DisableStars"] = setting.Repository.DisableStars
213-
ctx.Data["EnableActions"] = setting.Actions.Enabled
213+
ctx.Data["EnableActions"] = setting.Actions.Enabled && !unit.TypeActions.UnitGlobalDisabled()
214214

215215
ctx.Data["ManifestData"] = setting.ManifestData
216-
217-
ctx.Data["UnitWikiGlobalDisabled"] = unit.TypeWiki.UnitGlobalDisabled()
218-
ctx.Data["UnitIssuesGlobalDisabled"] = unit.TypeIssues.UnitGlobalDisabled()
219-
ctx.Data["UnitPullsGlobalDisabled"] = unit.TypePullRequests.UnitGlobalDisabled()
220-
ctx.Data["UnitProjectsGlobalDisabled"] = unit.TypeProjects.UnitGlobalDisabled()
221-
ctx.Data["UnitActionsGlobalDisabled"] = unit.TypeActions.UnitGlobalDisabled()
222-
223216
ctx.Data["AllLangs"] = translation.AllLangs()
224217

225218
next.ServeHTTP(ctx.Resp, ctx.Req)

‎templates/base/head_navbar.tmpl

+3-3
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,13 @@
3535
{{if and .IsSigned .MustChangePassword}}
3636
{{/* No links */}}
3737
{{else if .IsSigned}}
38-
{{if not .UnitIssuesGlobalDisabled}}
38+
{{if not ctx.Consts.RepoUnitTypeIssues.UnitGlobalDisabled}}
3939
<a class="item{{if .PageIsIssues}} active{{end}}" href="{{AppSubUrl}}/issues">{{ctx.Locale.Tr "issues"}}</a>
4040
{{end}}
41-
{{if not .UnitPullsGlobalDisabled}}
41+
{{if not ctx.Consts.RepoUnitTypePullRequests.UnitGlobalDisabled}}
4242
<a class="item{{if .PageIsPulls}} active{{end}}" href="{{AppSubUrl}}/pulls">{{ctx.Locale.Tr "pull_requests"}}</a>
4343
{{end}}
44-
{{if not (and .UnitIssuesGlobalDisabled .UnitPullsGlobalDisabled)}}
44+
{{if not (and ctx.Consts.RepoUnitTypeIssues.UnitGlobalDisabled ctx.Consts.RepoUnitTypePullRequests.UnitGlobalDisabled)}}
4545
{{if .ShowMilestonesDashboardPage}}
4646
<a class="item{{if .PageIsMilestonesDashboard}} active{{end}}" href="{{AppSubUrl}}/milestones">{{ctx.Locale.Tr "milestones"}}</a>
4747
{{end}}

‎templates/repo/header.tmpl

+2-2
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@
162162
</a>
163163
{{end}}
164164

165-
{{if and .EnableActions (not .UnitActionsGlobalDisabled) (.Permission.CanRead ctx.Consts.RepoUnitTypeActions)}}
165+
{{if and .EnableActions (.Permission.CanRead ctx.Consts.RepoUnitTypeActions)}}
166166
<a class="{{if .PageIsActions}}active {{end}}item" href="{{.RepoLink}}/actions">
167167
{{svg "octicon-play"}} {{ctx.Locale.Tr "actions.actions"}}
168168
{{if .Repository.NumOpenActionRuns}}
@@ -178,7 +178,7 @@
178178
{{end}}
179179

180180
{{$projectsUnit := .Repository.MustGetUnit $.Context ctx.Consts.RepoUnitTypeProjects}}
181-
{{if and (not .UnitProjectsGlobalDisabled) (.Permission.CanRead ctx.Consts.RepoUnitTypeProjects) ($projectsUnit.ProjectsConfig.IsProjectsAllowed "repo")}}
181+
{{if and (not ctx.Consts.RepoUnitTypeProjects.UnitGlobalDisabled) (.Permission.CanRead ctx.Consts.RepoUnitTypeProjects) ($projectsUnit.ProjectsConfig.IsProjectsAllowed "repo")}}
182182
<a href="{{.RepoLink}}/projects" class="{{if .IsProjectsPage}}active {{end}}item">
183183
{{svg "octicon-project"}} {{ctx.Locale.Tr "repo.projects"}}
184184
{{if .Repository.NumOpenProjects}}

‎templates/repo/issue/view_content/comments.tmpl

-2
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,6 @@
574574
{{template "repo/commits_list_small" dict "comment" . "root" $}}
575575
{{end}}
576576
{{else if eq .Type 30}}
577-
{{if not $.UnitProjectsGlobalDisabled}}
578577
<div class="timeline-item event" id="{{.HashTag}}">
579578
<span class="badge">{{svg "octicon-project"}}</span>
580579
{{template "shared/user/avatarlink" dict "user" .Poster}}
@@ -599,7 +598,6 @@
599598
{{end}}
600599
</span>
601600
</div>
602-
{{end}}
603601
{{else if eq .Type 32}}
604602
<div class="timeline-item-group">
605603
<div class="timeline-item event" id="{{.HashTag}}">

‎templates/repo/issue/view_content/context_menu.tmpl

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
{{if not .ctxData.Repository.IsArchived}}
1616
{{$needDivider = true}}
1717
<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>
18-
{{if not .ctxData.UnitIssuesGlobalDisabled}}
18+
{{if not ctx.Consts.RepoUnitTypeIssues.UnitGlobalDisabled}}
1919
<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>
2020
{{end}}
2121
{{if or .ctxData.Permission.IsAdmin .IsCommentPoster .ctxData.HasIssuesOrPullsWritePermission}}

‎templates/repo/settings/navbar.tmpl

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
</a>
3636
{{end}}
3737
{{end}}
38-
{{if and .EnableActions (not .UnitActionsGlobalDisabled) (.Permission.CanRead ctx.Consts.RepoUnitTypeActions)}}
38+
{{if and .EnableActions (.Permission.CanRead ctx.Consts.RepoUnitTypeActions)}}
3939
<details class="item toggleable-item" {{if or .PageIsSharedSettingsRunners .PageIsSharedSettingsSecrets .PageIsSharedSettingsVariables}}open{{end}}>
4040
<summary>{{ctx.Locale.Tr "actions.actions"}}</summary>
4141
<div class="menu">

‎templates/user/dashboard/navbar.tmpl

+3-3
Original file line numberDiff line numberDiff line change
@@ -81,17 +81,17 @@
8181
<a class="{{if .PageIsNews}}active {{end}}item tw-ml-auto" href="{{.ContextUser.DashboardLink}}{{if .Team}}/{{PathEscape .Team.Name}}{{end}}">
8282
{{svg "octicon-rss"}}&nbsp;{{ctx.Locale.Tr "activities"}}
8383
</a>
84-
{{if not .UnitIssuesGlobalDisabled}}
84+
{{if not ctx.Consts.RepoUnitTypeIssues.UnitGlobalDisabled}}
8585
<a class="{{if .PageIsIssues}}active {{end}}item" href="{{.ContextUser.OrganisationLink}}/issues{{if .Team}}/{{PathEscape .Team.Name}}{{end}}">
8686
{{svg "octicon-issue-opened"}}&nbsp;{{ctx.Locale.Tr "issues"}}
8787
</a>
8888
{{end}}
89-
{{if not .UnitPullsGlobalDisabled}}
89+
{{if not ctx.Consts.RepoUnitTypePullRequests.UnitGlobalDisabled}}
9090
<a class="{{if .PageIsPulls}}active {{end}}item" href="{{.ContextUser.OrganisationLink}}/pulls{{if .Team}}/{{PathEscape .Team.Name}}{{end}}">
9191
{{svg "octicon-git-pull-request"}}&nbsp;{{ctx.Locale.Tr "pull_requests"}}
9292
</a>
9393
{{end}}
94-
{{if and .ShowMilestonesDashboardPage (not (and .UnitIssuesGlobalDisabled .UnitPullsGlobalDisabled))}}
94+
{{if and .ShowMilestonesDashboardPage (not (and ctx.Consts.RepoUnitTypeIssues.UnitGlobalDisabled ctx.Consts.RepoUnitTypePullRequests.UnitGlobalDisabled))}}
9595
<a class="{{if .PageIsMilestonesDashboard}}active {{end}}item" href="{{.ContextUser.OrganisationLink}}/milestones{{if .Team}}/{{PathEscape .Team.Name}}{{end}}">
9696
{{svg "octicon-milestone"}}&nbsp;{{ctx.Locale.Tr "milestones"}}
9797
</a>

0 commit comments

Comments
 (0)