Enhancement: Align Gavatar and Federated Avatars options to be Enable

[markbrown87]

Feature Description

Admin settings for Picture and Avatar Configuration under Configuration are misaligned. Meaning, Gravatar is Disable Gravatar and Federate Avatars is Enable Federated Avatars. The proposed enhancement is a small refactor to align the two to be Enable for both (see screenshot).

Since this was a fast refactor, I implemented the approach of changing the naming and logic for disable gravatar to enable gravatar locally. However, I am not certain how the Gitea team would want to handle the database entry and migration from older instances to this proposed state. I assume there are standard procedures for migrating database entry names.

The alternative approach would be to keep all disable gravatar naming in the backend and flip the boolean when the control sends the value to the frontend. Of course, that comes with the confusion of the UI using Enable and the backend using Disable for naming convention for Gravatar.

Screenshots

[wxiaoguang]

Well, open-source and crowd-contributed.

You can see there are a lot of "DISABLE_xxx" https://github.com/go-gitea/gitea/blob/main/custom/conf/app.example.ini, most of them were added without careful design or thinking ....

[wxiaoguang]

I implemented the approach of changing the naming and logic for disable gravatar to enable gravatar locally

If your change is compatible with existing settings (won't break existing users: either by database migration, or flip the boolean in the fly) and could also help more "DISABLE_xxx" cases in the future (flip the boolean), I think it would be pretty good.

---

By the way, I think we can completely drop libravatar

#34284 (comment)

libravatar is introduced by gogs/gogs#3320 and maintained by @strk

It's really questionable any one is really using it

[markbrown87]

Though what I did locally was quick and dirty, targeting Gravatar exclusively, I will work on expanding it for others. I will put some time in removing libravatar as well.

[wxiaoguang]

Thank you very much.

---

Though what I did locally was quick and dirty, targeting Gravatar exclusively, I will work on expanding it for others.

My initial idea is to add a FlipBoolean field to config.Value, then it can use "enable" key in database and work with "disable" key in the legacy INI. Haven't really looked into it and thought more.

---

I will put some time in removing libravatar as well.

I think it doesn't need to be in this issue's scope. Maybe it can be a separate PR, in case some people have feedbacks and they still need it. (maybe wait for more feedbacks: #34284 (comment))

[markbrown87]

I spent some time yesterday modifying the config.Value such that the value was flipped whenever retrieved. This was accomplished by specifying .Invert() in the init function.

-			DisableGravatar:       config.ValueJSON[bool]("picture.disable_gravatar").WithFileConfig(config.CfgSecKey{Sec: "picture", Key: "DISABLE_GRAVATAR"}),
+			EnableGravatar:        config.ValueJSON[bool]("picture.disable_gravatar").WithFileConfig(config.CfgSecKey{Sec: "picture", Key: "DISABLE_GRAVATAR"}).Invert(),

However, the frontend sets based on input check box which required a flip there as well.

-	setting.Config().Picture.DisableGravatar.DynKey():       strconv.FormatBool(form.DisableGravatar),
+	setting.Config().Picture.EnableGravatar.DynKey():        strconv.FormatBool(!form.EnableGravatar), 

This meant the retrieval control flow and the setting control flow would both need to be flipped to accomplish flipping on the fly. This is the commit on my forked repo to flip on the fly, which is more confusing, in my opinion. It does, however, retain backwards compatibility for picture.disable_gravatar without requiring a database migration for the value.

---

I did spend the majority of my available time attempting to convert based on a composed "MigrateFrom" function such as

EnableGravatar:        config.ValueJSON[bool]("picture.enable_gravatar").MigrateFrom("picture.disable_gravatar").WithFileConfig(config.CfgSecKey{Sec: "picture", Key: "DISABLE_GRAVATAR"}).Invert(),

But found the use case of migrating from the old key value would cause for both picture.disable_gravatar and picture.enable_gravatar to be present in the database. This made it trickier to do purely from config.Value as the database would need to be updated only if picture.disable_gravatar was present at startup.

---

I am not familiar enough with the codebase, so I didn't make it far before shutting down for the night. Do you have any suggestions or pointers where I could begin implementing or using a database get, delete, and set? I feel it's more extensible if config.Value was composed with a setting to migrate the old dynKey to a new dynKey over flipping the value when retrieving or setting.

The link to the commit above does accomplish this ticket, but I feel it just serves to be more confusing when working in the code base and it's not very extensible.