Skip to content

Commit e5a8ebc

Browse files
lunnydelvhyp05327
authored
Require at least one unit to be enabled (#24189)
Don't remember why the previous decision that `Code` and `Release` are non-disable units globally. Since now every unit include `Code` could be disabled, maybe we should have a new rule that the repo should have at least one unit. So any unit could be disabled. Fixes #20960 Fixes #7525 --------- Co-authored-by: delvh <[email protected]> Co-authored-by: yp05327 <[email protected]>
1 parent 0ebabf3 commit e5a8ebc

File tree

9 files changed

+109
-75
lines changed

9 files changed

+109
-75
lines changed

‎models/repo.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@ var ItemsPerPage = 40
4040

4141
// Init initialize model
4242
func Init(ctx context.Context) error {
43-
unit.LoadUnitConfig()
43+
if err := unit.LoadUnitConfig(); err != nil {
44+
return err
45+
}
4446
return system_model.Init(ctx)
4547
}
4648

‎models/unit/unit.go‎

Lines changed: 11 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package unit
55

66
import (
7+
"errors"
78
"fmt"
89
"strings"
910

@@ -106,12 +107,6 @@ var (
106107
TypeExternalTracker,
107108
}
108109

109-
// MustRepoUnits contains the units could not be disabled currently
110-
MustRepoUnits = []Type{
111-
TypeCode,
112-
TypeReleases,
113-
}
114-
115110
// DisabledRepoUnits contains the units that have been globally disabled
116111
DisabledRepoUnits = []Type{}
117112
)
@@ -122,18 +117,13 @@ func validateDefaultRepoUnits(defaultUnits, settingDefaultUnits []Type) []Type {
122117

123118
// Use setting if not empty
124119
if len(settingDefaultUnits) > 0 {
125-
// MustRepoUnits required as default
126-
units = make([]Type, len(MustRepoUnits))
127-
copy(units, MustRepoUnits)
120+
units = make([]Type, 0, len(settingDefaultUnits))
128121
for _, settingUnit := range settingDefaultUnits {
129122
if !settingUnit.CanBeDefault() {
130123
log.Warn("Not allowed as default unit: %s", settingUnit.String())
131124
continue
132125
}
133-
// MustRepoUnits already added
134-
if settingUnit.CanDisable() {
135-
units = append(units, settingUnit)
136-
}
126+
units = append(units, settingUnit)
137127
}
138128
}
139129

@@ -150,30 +140,30 @@ func validateDefaultRepoUnits(defaultUnits, settingDefaultUnits []Type) []Type {
150140
}
151141

152142
// LoadUnitConfig load units from settings
153-
func LoadUnitConfig() {
143+
func LoadUnitConfig() error {
154144
var invalidKeys []string
155145
DisabledRepoUnits, invalidKeys = FindUnitTypes(setting.Repository.DisabledRepoUnits...)
156146
if len(invalidKeys) > 0 {
157147
log.Warn("Invalid keys in disabled repo units: %s", strings.Join(invalidKeys, ", "))
158148
}
159-
// Check that must units are not disabled
160-
for i, disabledU := range DisabledRepoUnits {
161-
if !disabledU.CanDisable() {
162-
log.Warn("Not allowed to global disable unit %s", disabledU.String())
163-
DisabledRepoUnits = append(DisabledRepoUnits[:i], DisabledRepoUnits[i+1:]...)
164-
}
165-
}
166149

167150
setDefaultRepoUnits, invalidKeys := FindUnitTypes(setting.Repository.DefaultRepoUnits...)
168151
if len(invalidKeys) > 0 {
169152
log.Warn("Invalid keys in default repo units: %s", strings.Join(invalidKeys, ", "))
170153
}
171154
DefaultRepoUnits = validateDefaultRepoUnits(DefaultRepoUnits, setDefaultRepoUnits)
155+
if len(DefaultRepoUnits) == 0 {
156+
return errors.New("no default repository units found")
157+
}
172158
setDefaultForkRepoUnits, invalidKeys := FindUnitTypes(setting.Repository.DefaultForkRepoUnits...)
173159
if len(invalidKeys) > 0 {
174160
log.Warn("Invalid keys in default fork repo units: %s", strings.Join(invalidKeys, ", "))
175161
}
176162
DefaultForkRepoUnits = validateDefaultRepoUnits(DefaultForkRepoUnits, setDefaultForkRepoUnits)
163+
if len(DefaultForkRepoUnits) == 0 {
164+
return errors.New("no default fork repository units found")
165+
}
166+
return nil
177167
}
178168

179169
// UnitGlobalDisabled checks if unit type is global disabled
@@ -186,16 +176,6 @@ func (u Type) UnitGlobalDisabled() bool {
186176
return false
187177
}
188178

189-
// CanDisable checks if this unit type can be disabled.
190-
func (u *Type) CanDisable() bool {
191-
for _, mu := range MustRepoUnits {
192-
if *u == mu {
193-
return false
194-
}
195-
}
196-
return true
197-
}
198-
199179
// CanBeDefault checks if the unit type can be a default repo unit
200180
func (u *Type) CanBeDefault() bool {
201181
for _, nadU := range NotAllowedDefaultRepoUnits {
@@ -216,11 +196,6 @@ type Unit struct {
216196
MaxAccessMode perm.AccessMode // The max access mode of the unit. i.e. Read means this unit can only be read.
217197
}
218198

219-
// CanDisable returns if this unit could be disabled.
220-
func (u *Unit) CanDisable() bool {
221-
return u.Type.CanDisable()
222-
}
223-
224199
// IsLessThan compares order of two units
225200
func (u Unit) IsLessThan(unit Unit) bool {
226201
if (u.Type == TypeExternalTracker || u.Type == TypeExternalWiki) && unit.Type != TypeExternalTracker && unit.Type != TypeExternalWiki {

‎models/unit/unit_test.go‎

Lines changed: 59 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,42 +12,84 @@ import (
1212
)
1313

1414
func TestLoadUnitConfig(t *testing.T) {
15-
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
16-
DisabledRepoUnits = disabledRepoUnits
17-
DefaultRepoUnits = defaultRepoUnits
18-
DefaultForkRepoUnits = defaultForkRepoUnits
19-
}(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
20-
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
21-
setting.Repository.DisabledRepoUnits = disabledRepoUnits
22-
setting.Repository.DefaultRepoUnits = defaultRepoUnits
23-
setting.Repository.DefaultForkRepoUnits = defaultForkRepoUnits
24-
}(setting.Repository.DisabledRepoUnits, setting.Repository.DefaultRepoUnits, setting.Repository.DefaultForkRepoUnits)
25-
2615
t.Run("regular", func(t *testing.T) {
16+
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
17+
DisabledRepoUnits = disabledRepoUnits
18+
DefaultRepoUnits = defaultRepoUnits
19+
DefaultForkRepoUnits = defaultForkRepoUnits
20+
}(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
21+
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
22+
setting.Repository.DisabledRepoUnits = disabledRepoUnits
23+
setting.Repository.DefaultRepoUnits = defaultRepoUnits
24+
setting.Repository.DefaultForkRepoUnits = defaultForkRepoUnits
25+
}(setting.Repository.DisabledRepoUnits, setting.Repository.DefaultRepoUnits, setting.Repository.DefaultForkRepoUnits)
26+
2727
setting.Repository.DisabledRepoUnits = []string{"repo.issues"}
2828
setting.Repository.DefaultRepoUnits = []string{"repo.code", "repo.releases", "repo.issues", "repo.pulls"}
2929
setting.Repository.DefaultForkRepoUnits = []string{"repo.releases"}
30-
LoadUnitConfig()
30+
assert.NoError(t, LoadUnitConfig())
3131
assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnits)
3232
assert.Equal(t, []Type{TypeCode, TypeReleases, TypePullRequests}, DefaultRepoUnits)
33-
assert.Equal(t, []Type{TypeCode, TypeReleases}, DefaultForkRepoUnits)
33+
assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits)
3434
})
3535
t.Run("invalid", func(t *testing.T) {
36+
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
37+
DisabledRepoUnits = disabledRepoUnits
38+
DefaultRepoUnits = defaultRepoUnits
39+
DefaultForkRepoUnits = defaultForkRepoUnits
40+
}(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
41+
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
42+
setting.Repository.DisabledRepoUnits = disabledRepoUnits
43+
setting.Repository.DefaultRepoUnits = defaultRepoUnits
44+
setting.Repository.DefaultForkRepoUnits = defaultForkRepoUnits
45+
}(setting.Repository.DisabledRepoUnits, setting.Repository.DefaultRepoUnits, setting.Repository.DefaultForkRepoUnits)
46+
3647
setting.Repository.DisabledRepoUnits = []string{"repo.issues", "invalid.1"}
3748
setting.Repository.DefaultRepoUnits = []string{"repo.code", "invalid.2", "repo.releases", "repo.issues", "repo.pulls"}
3849
setting.Repository.DefaultForkRepoUnits = []string{"invalid.3", "repo.releases"}
39-
LoadUnitConfig()
50+
assert.NoError(t, LoadUnitConfig())
4051
assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnits)
4152
assert.Equal(t, []Type{TypeCode, TypeReleases, TypePullRequests}, DefaultRepoUnits)
42-
assert.Equal(t, []Type{TypeCode, TypeReleases}, DefaultForkRepoUnits)
53+
assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits)
4354
})
4455
t.Run("duplicate", func(t *testing.T) {
56+
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
57+
DisabledRepoUnits = disabledRepoUnits
58+
DefaultRepoUnits = defaultRepoUnits
59+
DefaultForkRepoUnits = defaultForkRepoUnits
60+
}(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
61+
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
62+
setting.Repository.DisabledRepoUnits = disabledRepoUnits
63+
setting.Repository.DefaultRepoUnits = defaultRepoUnits
64+
setting.Repository.DefaultForkRepoUnits = defaultForkRepoUnits
65+
}(setting.Repository.DisabledRepoUnits, setting.Repository.DefaultRepoUnits, setting.Repository.DefaultForkRepoUnits)
66+
4567
setting.Repository.DisabledRepoUnits = []string{"repo.issues", "repo.issues"}
4668
setting.Repository.DefaultRepoUnits = []string{"repo.code", "repo.releases", "repo.issues", "repo.pulls", "repo.code"}
4769
setting.Repository.DefaultForkRepoUnits = []string{"repo.releases", "repo.releases"}
48-
LoadUnitConfig()
70+
assert.NoError(t, LoadUnitConfig())
4971
assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnits)
5072
assert.Equal(t, []Type{TypeCode, TypeReleases, TypePullRequests}, DefaultRepoUnits)
51-
assert.Equal(t, []Type{TypeCode, TypeReleases}, DefaultForkRepoUnits)
73+
assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits)
74+
})
75+
t.Run("empty_default", func(t *testing.T) {
76+
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
77+
DisabledRepoUnits = disabledRepoUnits
78+
DefaultRepoUnits = defaultRepoUnits
79+
DefaultForkRepoUnits = defaultForkRepoUnits
80+
}(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
81+
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
82+
setting.Repository.DisabledRepoUnits = disabledRepoUnits
83+
setting.Repository.DefaultRepoUnits = defaultRepoUnits
84+
setting.Repository.DefaultForkRepoUnits = defaultForkRepoUnits
85+
}(setting.Repository.DisabledRepoUnits, setting.Repository.DefaultRepoUnits, setting.Repository.DefaultForkRepoUnits)
86+
87+
setting.Repository.DisabledRepoUnits = []string{"repo.issues", "repo.issues"}
88+
setting.Repository.DefaultRepoUnits = []string{}
89+
setting.Repository.DefaultForkRepoUnits = []string{"repo.releases", "repo.releases"}
90+
assert.NoError(t, LoadUnitConfig())
91+
assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnits)
92+
assert.ElementsMatch(t, []Type{TypeCode, TypePullRequests, TypeReleases, TypeWiki, TypePackages, TypeProjects}, DefaultRepoUnits)
93+
assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits)
5294
})
5395
}

‎options/locale/locale_en-US.ini‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2004,6 +2004,7 @@ settings.delete_notices_2 = - This operation will permanently delete the <strong
20042004
settings.delete_notices_fork_1 = - Forks of this repository will become independent after deletion.
20052005
settings.deletion_success = The repository has been deleted.
20062006
settings.update_settings_success = The repository settings have been updated.
2007+
settings.update_settings_no_unit = The repository should allow at least some sort of interaction.
20072008
settings.confirm_delete = Delete Repository
20082009
settings.add_collaborator = Add Collaborator
20092010
settings.add_collaborator_success = The collaborator has been added.

‎routers/api/v1/repo/repo.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -970,9 +970,11 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error {
970970
}
971971
}
972972

973-
if err := repo_model.UpdateRepositoryUnits(repo, units, deleteUnitTypes); err != nil {
974-
ctx.Error(http.StatusInternalServerError, "UpdateRepositoryUnits", err)
975-
return err
973+
if len(units)+len(deleteUnitTypes) > 0 {
974+
if err := repo_model.UpdateRepositoryUnits(repo, units, deleteUnitTypes); err != nil {
975+
ctx.Error(http.StatusInternalServerError, "UpdateRepositoryUnits", err)
976+
return err
977+
}
976978
}
977979

978980
log.Trace("Repository advanced settings updated: %s/%s", owner.Name, repo.Name)

‎routers/web/repo/setting.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,12 @@ func SettingsPost(ctx *context.Context) {
536536
deleteUnitTypes = append(deleteUnitTypes, unit_model.TypePullRequests)
537537
}
538538

539+
if len(units) == 0 {
540+
ctx.Flash.Error(ctx.Tr("repo.settings.update_settings_no_unit"))
541+
ctx.Redirect(ctx.Repo.RepoLink + "/settings")
542+
return
543+
}
544+
539545
if err := repo_model.UpdateRepositoryUnits(repo, units, deleteUnitTypes); err != nil {
540546
ctx.ServerError("UpdateRepositoryUnits", err)
541547
return

‎routers/web/web.go‎

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,27 @@ func registerRoutes(m *web.Route) {
261261
}
262262
}
263263

264+
reqUnitAccess := func(unitType unit.Type, accessMode perm.AccessMode) func(ctx *context.Context) {
265+
return func(ctx *context.Context) {
266+
if unitType.UnitGlobalDisabled() {
267+
ctx.NotFound(unitType.String(), nil)
268+
return
269+
}
270+
271+
if ctx.ContextUser == nil {
272+
ctx.NotFound(unitType.String(), nil)
273+
return
274+
}
275+
276+
if ctx.ContextUser.IsOrganization() {
277+
if ctx.Org.Organization.UnitPermission(ctx, ctx.Doer, unitType) < accessMode {
278+
ctx.NotFound(unitType.String(), nil)
279+
return
280+
}
281+
}
282+
}
283+
}
284+
264285
addWebhookAddRoutes := func() {
265286
m.Get("/{type}/new", repo.WebhooksNew)
266287
m.Post("/gitea/new", web.Bind(forms.NewWebhookForm{}), repo.GiteaHooksNewPost)
@@ -334,7 +355,7 @@ func registerRoutes(m *web.Route) {
334355
m.Get("/users", explore.Users)
335356
m.Get("/users/sitemap-{idx}.xml", sitemapEnabled, explore.Users)
336357
m.Get("/organizations", explore.Organizations)
337-
m.Get("/code", explore.Code)
358+
m.Get("/code", reqUnitAccess(unit.TypeCode, perm.AccessModeRead), explore.Code)
338359
m.Get("/topics/search", explore.TopicSearch)
339360
}, ignExploreSignIn)
340361
m.Group("/issues", func() {
@@ -649,21 +670,6 @@ func registerRoutes(m *web.Route) {
649670
}
650671
}
651672

652-
reqUnitAccess := func(unitType unit.Type, accessMode perm.AccessMode) func(ctx *context.Context) {
653-
return func(ctx *context.Context) {
654-
if ctx.ContextUser == nil {
655-
ctx.NotFound(unitType.String(), nil)
656-
return
657-
}
658-
if ctx.ContextUser.IsOrganization() {
659-
if ctx.Org.Organization.UnitPermission(ctx, ctx.Doer, unitType) < accessMode {
660-
ctx.NotFound(unitType.String(), nil)
661-
return
662-
}
663-
}
664-
}
665-
}
666-
667673
// ***** START: Organization *****
668674
m.Group("/org", func() {
669675
m.Group("/{org}", func() {

‎templates/explore/navbar.tmpl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
<a class="{{if .PageIsExploreOrganizations}}active {{end}}item" href="{{AppSubUrl}}/explore/organizations">
1111
{{svg "octicon-organization"}} {{.locale.Tr "explore.organizations"}}
1212
</a>
13-
{{if .IsRepoIndexerEnabled}}
13+
{{if and (not $.UnitTypeCode.UnitGlobalDisabled) .IsRepoIndexerEnabled}}
1414
<a class="{{if .PageIsExploreCode}}active {{end}}item" href="{{AppSubUrl}}/explore/code">
1515
{{svg "octicon-code"}} {{.locale.Tr "explore.code"}}
1616
</a>

‎templates/user/profile.tmpl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@
135135
{{svg "octicon-package"}} {{.locale.Tr "packages.title"}}
136136
</a>
137137
{{end}}
138-
{{if .IsRepoIndexerEnabled}}
138+
{{if and (not $.UnitTypeCode.UnitGlobalDisabled) .IsRepoIndexerEnabled}}
139139
<a class='{{if eq .TabName "code"}}active {{end}}item' href="{{.ContextUser.HomeLink}}/-/code">
140140
{{svg "octicon-code"}} {{.locale.Tr "user.code"}}
141141
</a>

0 commit comments

Comments
 (0)