Skip to content

Commit 589712d

Browse files
markbrown87markbrown87
committed
feat: inverted disable_gravatar logic to enable_gravatar
Frontend still interacts directly with the database entry name `picture.disable_gravatar` so logic needs flipped when writing, but logic to read automatically flips based on config.Invert() being called during init or INI read.
1 parent ebd88af commit 589712d

File tree

18 files changed

+100
-42
lines changed

18 files changed

+100
-42
lines changed

‎models/avatars/avatar.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,8 +216,8 @@ func generateEmailAvatarLink(ctx context.Context, email string, size int, final
216216
return urlStr
217217
}
218218

219-
disableGravatar := setting.Config().Picture.DisableGravatar.Value(ctx)
220-
if !disableGravatar {
219+
enableGravatar := setting.Config().Picture.EnableGravatar.Value(ctx)
220+
if enableGravatar {
221221
// copy GravatarSourceURL, because we will modify its Path.
222222
avatarURLCopy := *avatarSetting.gravatarSourceURL
223223
avatarURLCopy.Path = path.Join(avatarURLCopy.Path, HashEmail(email))

‎models/avatars/avatar_test.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,14 @@ const gravatarSource = "https://secure.gravatar.com/avatar/"
1919
func disableGravatar(t *testing.T) {
2020
err := system_model.SetSettings(t.Context(), map[string]string{setting.Config().Picture.EnableFederatedAvatar.DynKey(): "false"})
2121
assert.NoError(t, err)
22-
err = system_model.SetSettings(t.Context(), map[string]string{setting.Config().Picture.DisableGravatar.DynKey(): "true"})
22+
// EnableGravatar.DynKey == picture.disable_gravatar for backwards compatability; .Value will flip correctly but the true value here is misleading
23+
err = system_model.SetSettings(t.Context(), map[string]string{setting.Config().Picture.EnableGravatar.DynKey(): "true"})
2324
assert.NoError(t, err)
2425
}
2526

2627
func enableGravatar(t *testing.T) {
27-
err := system_model.SetSettings(t.Context(), map[string]string{setting.Config().Picture.DisableGravatar.DynKey(): "false"})
28+
// EnableGravatar.DynKey == picture.disable_gravatar for backwards compatability; .Value will flip correctly but the false value here is misleading
29+
err := system_model.SetSettings(t.Context(), map[string]string{setting.Config().Picture.EnableGravatar.DynKey(): "false"})
2830
assert.NoError(t, err)
2931
setting.GravatarSource = gravatarSource
3032
}

‎models/user/avatar.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,12 @@ func (u *User) AvatarLinkWithSize(ctx context.Context, size int) string {
6969
useLocalAvatar := false
7070
autoGenerateAvatar := false
7171

72-
disableGravatar := setting.Config().Picture.DisableGravatar.Value(ctx)
72+
enableGravatar := setting.Config().Picture.EnableGravatar.Value(ctx)
7373

7474
switch {
75-
case u.UseCustomAvatar:
75+
case u.UseCustomAvatar, enableGravatar:
7676
useLocalAvatar = true
77-
case disableGravatar, setting.OfflineMode:
77+
case setting.OfflineMode:
7878
useLocalAvatar = true
7979
autoGenerateAvatar = true
8080
}

‎modules/setting/config.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111
)
1212

1313
type PictureStruct struct {
14-
DisableGravatar *config.Value[bool]
14+
EnableGravatar *config.Value[bool]
1515
EnableFederatedAvatar *config.Value[bool]
1616
}
1717

@@ -66,7 +66,7 @@ func initDefaultConfig() {
6666
config.SetCfgSecKeyGetter(&cfgSecKeyGetter{})
6767
defaultConfig = &ConfigStruct{
6868
Picture: &PictureStruct{
69-
DisableGravatar: config.ValueJSON[bool]("picture.disable_gravatar").WithFileConfig(config.CfgSecKey{Sec: "picture", Key: "DISABLE_GRAVATAR"}),
69+
EnableGravatar: config.ValueJSON[bool]("picture.disable_gravatar").WithFileConfig(config.CfgSecKey{Sec: "picture", Key: "DISABLE_GRAVATAR"}).Invert(),
7070
EnableFederatedAvatar: config.ValueJSON[bool]("picture.enable_federated_avatar").WithFileConfig(config.CfgSecKey{Sec: "picture", Key: "ENABLE_FEDERATED_AVATAR"}),
7171
},
7272
Repository: &RepositoryStruct{

‎modules/setting/config/value.go‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@ type Value[T any] struct {
2222
cfgSecKey CfgSecKey
2323
dynKey string
2424

25-
def, value T
26-
revision int
25+
def, value T
26+
revision int
27+
flipBoolean bool
2728
}
2829

2930
func (value *Value[T]) parse(key, valStr string) (v T) {
@@ -33,6 +34,17 @@ func (value *Value[T]) parse(key, valStr string) (v T) {
3334
log.Error("Unable to unmarshal json config for key %q, err: %v", key, err)
3435
}
3536
}
37+
38+
if value.flipBoolean {
39+
// if value is of type bool
40+
if _, ok := any(v).(bool); ok {
41+
// invert the boolean value upon retrieval
42+
v = any(!any(v).(bool)).(T)
43+
} else {
44+
log.Warn("Ignoring attempt to invert key '%q' for non boolean type", key)
45+
}
46+
}
47+
3648
return v
3749
}
3850

@@ -93,6 +105,11 @@ func (value *Value[T]) WithFileConfig(cfgSecKey CfgSecKey) *Value[T] {
93105
return value
94106
}
95107

108+
func (value *Value[bool]) Invert() *Value[bool] {
109+
value.flipBoolean = true
110+
return value
111+
}
112+
96113
func ValueJSON[T any](dynKey string) *Value[T] {
97114
return &Value[T]{dynKey: dynKey}
98115
}

‎modules/setting/config/value_test.go‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Copyright 2023 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package config
5+
6+
import (
7+
"testing"
8+
)
9+
10+
func TestValue_parse(t *testing.T) {
11+
tests := []struct {
12+
name string // description of this test case
13+
// Named input parameters for target function.
14+
key string
15+
valStr string
16+
want bool
17+
}{
18+
{
19+
name: "Parse Invert Retrieval",
20+
key: "picture.disable_gravatar",
21+
valStr: "false",
22+
want: true,
23+
},
24+
}
25+
for _, tt := range tests {
26+
t.Run(tt.name, func(t *testing.T) {
27+
value := ValueJSON[bool]("picture.disable_gravatar").WithFileConfig(CfgSecKey{Sec: "picture", Key: "DISABLE_GRAVATAR"}).Invert()
28+
got := value.parse(tt.key, tt.valStr)
29+
30+
if got != tt.want {
31+
t.Errorf("parse() = %v, want %v", got, tt.want)
32+
}
33+
})
34+
}
35+
}

‎modules/setting/picture.go‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ var (
2323
}
2424

2525
GravatarSource string
26-
DisableGravatar bool // Depreciated: migrated to database
26+
EnableGravatar bool // Depreciated: migrated to database
2727
EnableFederatedAvatar bool // Depreciated: migrated to database
2828

2929
RepoAvatar = struct {
@@ -65,9 +65,9 @@ func loadAvatarsFrom(rootCfg ConfigProvider) error {
6565
GravatarSource = source
6666
}
6767

68-
DisableGravatar = sec.Key("DISABLE_GRAVATAR").MustBool(GetDefaultDisableGravatar())
68+
EnableGravatar = !sec.Key("DISABLE_GRAVATAR").MustBool(GetDefaultDisableGravatar())
6969
deprecatedSettingDB(rootCfg, "", "DISABLE_GRAVATAR")
70-
EnableFederatedAvatar = sec.Key("ENABLE_FEDERATED_AVATAR").MustBool(GetDefaultEnableFederatedAvatar(DisableGravatar))
70+
EnableFederatedAvatar = sec.Key("ENABLE_FEDERATED_AVATAR").MustBool(GetDefaultEnableFederatedAvatar(EnableGravatar))
7171
deprecatedSettingDB(rootCfg, "", "ENABLE_FEDERATED_AVATAR")
7272

7373
return nil
@@ -77,14 +77,12 @@ func GetDefaultDisableGravatar() bool {
7777
return OfflineMode
7878
}
7979

80-
func GetDefaultEnableFederatedAvatar(disableGravatar bool) bool {
80+
func GetDefaultEnableFederatedAvatar(enableGravatar bool) bool {
8181
v := !InstallLock
82-
if OfflineMode {
83-
v = false
84-
}
85-
if disableGravatar {
82+
if OfflineMode || !enableGravatar {
8683
v = false
8784
}
85+
8886
return v
8987
}
9088

‎options/locale/locale_en-US.ini‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -316,8 +316,8 @@ mail_notify = Enable Email Notifications
316316
server_service_title = Server and Third-Party Service Settings
317317
offline_mode = Enable Local Mode
318318
offline_mode_popup = Disable third-party content delivery networks and serve all resources locally.
319-
disable_gravatar = Disable Gravatar
320-
disable_gravatar_popup = Disable Gravatar and third-party avatar sources. A default avatar will be used unless a user locally uploads an avatar.
319+
enable_gravatar = Enable Gravatar
320+
enable_gravatar_popup = Enable Gravatar and third-party avatar sources. A default avatar will be used unless a user locally uploads an avatar.
321321
federated_avatar_lookup = Enable Federated Avatars
322322
federated_avatar_lookup_popup = Enable federated avatar lookup using Libravatar.
323323
disable_registration = Disable Self-Registration
@@ -3433,7 +3433,7 @@ config.cookie_life_time = Cookie Life Time
34333433
34343434
config.picture_config = Picture and Avatar Configuration
34353435
config.picture_service = Picture Service
3436-
config.disable_gravatar = Disable Gravatar
3436+
config.enable_gravatar = Enable Gravatar
34373437
config.enable_federated_avatar = Enable Federated Avatars
34383438
config.open_with_editor_app_help = The "Open with" editors for the clone menu. If left empty, the default will be used. Expand to see the default.
34393439
config.git_guide_remote_name = Repository remote name for git commands in the guide

‎routers/install/install.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ func Install(ctx *context.Context) {
136136

137137
// Server and other services settings
138138
form.OfflineMode = setting.OfflineMode
139-
form.DisableGravatar = setting.DisableGravatar // when installing, there is no database connection so that given a default value
139+
form.EnableGravatar = setting.EnableGravatar // when installing, there is no database connection so that given a default value
140140
form.EnableFederatedAvatar = setting.EnableFederatedAvatar // when installing, there is no database connection so that given a default value
141141

142142
form.EnableOpenIDSignIn = setting.Service.EnableOpenIDSignIn
@@ -427,7 +427,7 @@ func SubmitInstall(ctx *context.Context) {
427427

428428
cfg.Section("server").Key("OFFLINE_MODE").SetValue(strconv.FormatBool(form.OfflineMode))
429429
if err := system_model.SetSettings(ctx, map[string]string{
430-
setting.Config().Picture.DisableGravatar.DynKey(): strconv.FormatBool(form.DisableGravatar),
430+
setting.Config().Picture.EnableGravatar.DynKey(): strconv.FormatBool(!form.EnableGravatar), // invert value as it is stored as disable_gravatar for backwards compatability
431431
setting.Config().Picture.EnableFederatedAvatar.DynKey(): strconv.FormatBool(form.EnableFederatedAvatar),
432432
}); err != nil {
433433
ctx.RenderWithErr(ctx.Tr("install.save_config_failed", err), tplInstall, &form)

‎routers/web/admin/config.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,11 @@ func ChangeConfig(ctx *context.Context) {
203203
return json.Marshal(b)
204204
}
205205

206+
marshalBoolInvert := func(v string) ([]byte, error) {
207+
b, _ := strconv.ParseBool(v)
208+
return json.Marshal(!b)
209+
}
210+
206211
marshalString := func(emptyDefault string) func(v string) ([]byte, error) {
207212
return func(v string) ([]byte, error) {
208213
return json.Marshal(util.IfZero(v, emptyDefault))
@@ -230,8 +235,9 @@ func ChangeConfig(ctx *context.Context) {
230235
}
231236
return json.Marshal(openWithEditorApps)
232237
}
238+
233239
marshallers := map[string]func(string) ([]byte, error){
234-
cfg.Picture.DisableGravatar.DynKey(): marshalBool,
240+
cfg.Picture.EnableGravatar.DynKey(): marshalBoolInvert, // Invert for backwards compatability with old database semantics
235241
cfg.Picture.EnableFederatedAvatar.DynKey(): marshalBool,
236242
cfg.Repository.OpenWithEditorApps.DynKey(): marshalOpenWithApps,
237243
cfg.Repository.GitGuideRemoteName.DynKey(): marshalString(cfg.Repository.GitGuideRemoteName.DefaultValue()),

0 commit comments

Comments
 (0)