From a1486b0ee4feb569876ec65d9ba2030e70074557 Mon Sep 17 00:00:00 2001 From: "Panagiotis \"Ivory\" Vasilopoulos" Date: Sat, 15 Feb 2025 13:07:15 +0000 Subject: [PATCH] feat: add pronoun privacy option (#6773) This commit contains UI changes, tests and migrations for a feature that lets users optionally hide their pronouns from the general public. This is useful if a person wants to disclose that information to a smaller set of people on a local instance belonging to a local community/association. Co-authored-by: Gusted Co-authored-by: Beowulf Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6773 Reviewed-by: Gusted Co-authored-by: Panagiotis "Ivory" Vasilopoulos Co-committed-by: Panagiotis "Ivory" Vasilopoulos --- models/fixtures/user.yml | 2 + models/forgejo_migrations/migrate.go | 2 + models/forgejo_migrations/v29.go | 15 ++++ models/user/user.go | 11 +++ models/user/user_test.go | 39 ++++++++++ modules/structs/user.go | 2 + options/locale/locale_en-US.ini | 4 +- routers/api/v1/user/settings.go | 1 + routers/common/auth.go | 1 + routers/web/user/setting/profile.go | 1 + services/convert/user.go | 3 +- services/forms/user_form.go | 1 + services/user/update.go | 7 ++ templates/shared/user/profile_big_avatar.tmpl | 2 +- templates/swagger/v1_json.tmpl | 8 ++ templates/user/settings/profile.tmpl | 6 ++ tests/integration/user_test.go | 78 +++++++++++++------ 17 files changed, 158 insertions(+), 25 deletions(-) create mode 100644 models/forgejo_migrations/v29.go diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index c23abbb17e..630505b8b4 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -45,6 +45,7 @@ full_name: ' < Ur Tw >< ' email: user2@example.com keep_email_private: true + keep_pronouns_private: true email_notifications_preference: enabled passwd: ZogKvWdyEx:password passwd_hash_algo: dummy @@ -350,6 +351,7 @@ full_name: User Ten email: user10@example.com keep_email_private: false + keep_pronouns_private: true email_notifications_preference: enabled passwd: ZogKvWdyEx:password passwd_hash_algo: dummy diff --git a/models/forgejo_migrations/migrate.go b/models/forgejo_migrations/migrate.go index 1450ad3c54..4bc17a4288 100644 --- a/models/forgejo_migrations/migrate.go +++ b/models/forgejo_migrations/migrate.go @@ -92,6 +92,8 @@ var migrations = []*Migration{ NewMigration("Add `hash_blake2b` column to `package_blob` table", AddHashBlake2bToPackageBlob), // v27 -> v28 NewMigration("Add `created_unix` column to `user_redirect` table", AddCreatedUnixToRedirect), + // v28 -> v29 + NewMigration("Add pronoun privacy settings to user", AddHidePronounsOptionToUser), } // GetCurrentDBVersion returns the current Forgejo database version. diff --git a/models/forgejo_migrations/v29.go b/models/forgejo_migrations/v29.go new file mode 100644 index 0000000000..cba888d2ec --- /dev/null +++ b/models/forgejo_migrations/v29.go @@ -0,0 +1,15 @@ +// Copyright 2024 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package forgejo_migrations //nolint:revive + +import "xorm.io/xorm" + +func AddHidePronounsOptionToUser(x *xorm.Engine) error { + type User struct { + ID int64 `xorm:"pk autoincr"` + KeepPronounsPrivate bool `xorm:"NOT NULL DEFAULT false"` + } + + return x.Sync(&User{}) +} diff --git a/models/user/user.go b/models/user/user.go index f986ac5482..846190a648 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -154,6 +154,7 @@ type User struct { DiffViewStyle string `xorm:"NOT NULL DEFAULT ''"` Theme string `xorm:"NOT NULL DEFAULT ''"` KeepActivityPrivate bool `xorm:"NOT NULL DEFAULT false"` + KeepPronounsPrivate bool `xorm:"NOT NULL DEFAULT false"` EnableRepoUnitHints bool `xorm:"NOT NULL DEFAULT true"` } @@ -500,6 +501,16 @@ func (u *User) GetCompleteName() string { return u.Name } +// GetPronouns returns an empty string, if the user has set to keep his +// pronouns private from non-logged in users, otherwise the pronouns +// are returned. +func (u *User) GetPronouns(signed bool) string { + if u.KeepPronounsPrivate && !signed { + return "" + } + return u.Pronouns +} + func gitSafeName(name string) string { return strings.TrimSpace(strings.NewReplacer("\n", "", "<", "", ">", "").Replace(name)) } diff --git a/models/user/user_test.go b/models/user/user_test.go index 263c38933a..e3479943e3 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -795,3 +795,42 @@ func TestGetInactiveUsers(t *testing.T) { require.NoError(t, err) require.Empty(t, users) } + +func TestPronounsPrivacy(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + t.Run("EmptyPronounsIfNoneSet", func(t *testing.T) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + user.Pronouns = "" + user.KeepPronounsPrivate = false + + assert.Equal(t, "", user.GetPronouns(false)) + }) + t.Run("EmptyPronounsIfSetButPrivateAndNotLoggedIn", func(t *testing.T) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + user.Pronouns = "any" + user.KeepPronounsPrivate = true + + assert.Equal(t, "", user.GetPronouns(false)) + }) + t.Run("ReturnPronounsIfSetAndNotPrivateAndNotLoggedIn", func(t *testing.T) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + user.Pronouns = "any" + user.KeepPronounsPrivate = false + + assert.Equal(t, "any", user.GetPronouns(false)) + }) + t.Run("ReturnPronounsIfSetAndPrivateAndLoggedIn", func(t *testing.T) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + user.Pronouns = "any" + user.KeepPronounsPrivate = false + + assert.Equal(t, "any", user.GetPronouns(true)) + }) + t.Run("ReturnPronounsIfSetAndNotPrivateAndLoggedIn", func(t *testing.T) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + user.Pronouns = "any" + user.KeepPronounsPrivate = true + + assert.Equal(t, "any", user.GetPronouns(true)) + }) +} diff --git a/modules/structs/user.go b/modules/structs/user.go index f2747b1473..2dd3051c50 100644 --- a/modules/structs/user.go +++ b/modules/structs/user.go @@ -84,6 +84,7 @@ type UserSettings struct { EnableRepoUnitHints bool `json:"enable_repo_unit_hints"` // Privacy HideEmail bool `json:"hide_email"` + HidePronouns bool `json:"hide_pronouns"` HideActivity bool `json:"hide_activity"` } @@ -101,6 +102,7 @@ type UserSettingsOptions struct { EnableRepoUnitHints *bool `json:"enable_repo_unit_hints"` // Privacy HideEmail *bool `json:"hide_email"` + HidePronouns *bool `json:"hide_pronouns"` HideActivity *bool `json:"hide_activity"` } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 03f3af7415..ae6bc53924 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -853,6 +853,8 @@ email_preference_set_success = Email preference has been set successfully. add_openid_success = The new OpenID address has been added. keep_email_private = Hide email address keep_email_private_popup = Your email address will not be shown on your profile and will not be the default for commits made via the web interface, like file uploads, edits, and merge commits. Instead, a special address %s can be used to link commits to your account. This option will not affect existing commits. +keep_pronouns_private = Only show pronouns to authenticated users +keep_pronouns_private.description = This will hide your pronouns from visitors that are not logged in. openid_desc = OpenID lets you delegate authentication to an external provider. manage_ssh_keys = Manage SSH keys @@ -3935,4 +3937,4 @@ filepreview.lines = Lines %[1]d to %[2]d in %[3]s filepreview.truncated = Preview has been truncated [translation_meta] -test = This is a test string. It is not displayed in Forgejo UI but is used for testing purposes. Feel free to enter "ok" to save time (or a fun fact of your choice) to hit that sweet 100% completion mark :) \ No newline at end of file +test = This is a test string. It is not displayed in Forgejo UI but is used for testing purposes. Feel free to enter "ok" to save time (or a fun fact of your choice) to hit that sweet 100% completion mark :) diff --git a/routers/api/v1/user/settings.go b/routers/api/v1/user/settings.go index 173f06e474..67ab0dd964 100644 --- a/routers/api/v1/user/settings.go +++ b/routers/api/v1/user/settings.go @@ -63,6 +63,7 @@ func UpdateUserSettings(ctx *context.APIContext) { Theme: optional.FromPtr(form.Theme), DiffViewStyle: optional.FromPtr(form.DiffViewStyle), KeepEmailPrivate: optional.FromPtr(form.HideEmail), + KeepPronounsPrivate: optional.FromPtr(form.HidePronouns), KeepActivityPrivate: optional.FromPtr(form.HideActivity), EnableRepoUnitHints: optional.FromPtr(form.EnableRepoUnitHints), } diff --git a/routers/common/auth.go b/routers/common/auth.go index 115d65ed10..722c625e7b 100644 --- a/routers/common/auth.go +++ b/routers/common/auth.go @@ -31,6 +31,7 @@ func AuthShared(ctx *context.Base, sessionStore auth_service.SessionStore, authM ctx.Data["SignedUserID"] = ar.Doer.ID ctx.Data["IsAdmin"] = ar.Doer.IsAdmin } else { + ctx.Data["IsSigned"] = false ctx.Data["SignedUserID"] = int64(0) } return ar, nil diff --git a/routers/web/user/setting/profile.go b/routers/web/user/setting/profile.go index 818da9e3fa..33b2919d69 100644 --- a/routers/web/user/setting/profile.go +++ b/routers/web/user/setting/profile.go @@ -106,6 +106,7 @@ func ProfilePost(ctx *context.Context) { Location: optional.Some(form.Location), Visibility: optional.Some(form.Visibility), KeepActivityPrivate: optional.Some(form.KeepActivityPrivate), + KeepPronounsPrivate: optional.Some(form.KeepPronounsPrivate), } if err := user_service.UpdateUser(ctx, ctx.Doer, opts); err != nil { ctx.ServerError("UpdateUser", err) diff --git a/services/convert/user.go b/services/convert/user.go index 94a400de5d..7b6775dfb4 100644 --- a/services/convert/user.go +++ b/services/convert/user.go @@ -57,7 +57,7 @@ func toUser(ctx context.Context, user *user_model.User, signed, authed bool) *ap Created: user.CreatedUnix.AsTime(), Restricted: user.IsRestricted, Location: user.Location, - Pronouns: user.Pronouns, + Pronouns: user.GetPronouns(signed), Website: user.Website, Description: user.Description, // counter's @@ -97,6 +97,7 @@ func User2UserSettings(user *user_model.User) api.UserSettings { Description: user.Description, Theme: user.Theme, HideEmail: user.KeepEmailPrivate, + HidePronouns: user.KeepPronounsPrivate, HideActivity: user.KeepActivityPrivate, DiffViewStyle: user.DiffViewStyle, EnableRepoUnitHints: user.EnableRepoUnitHints, diff --git a/services/forms/user_form.go b/services/forms/user_form.go index 0d06f4b417..d76e97ceb1 100644 --- a/services/forms/user_form.go +++ b/services/forms/user_form.go @@ -224,6 +224,7 @@ type UpdateProfileForm struct { Biography string `binding:"MaxSize(255)"` Visibility structs.VisibleType KeepActivityPrivate bool + KeepPronounsPrivate bool } // Validate validates the fields diff --git a/services/user/update.go b/services/user/update.go index 26c90505c8..62c30ac05f 100644 --- a/services/user/update.go +++ b/services/user/update.go @@ -40,6 +40,7 @@ type UpdateOptions struct { SetLastLogin bool RepoAdminChangeTeamAccess optional.Option[bool] EnableRepoUnitHints optional.Option[bool] + KeepPronounsPrivate optional.Option[bool] } func UpdateUser(ctx context.Context, u *user_model.User, opts *UpdateOptions) error { @@ -97,6 +98,12 @@ func UpdateUser(ctx context.Context, u *user_model.User, opts *UpdateOptions) er cols = append(cols, "enable_repo_unit_hints") } + if opts.KeepPronounsPrivate.Has() { + u.KeepPronounsPrivate = opts.KeepPronounsPrivate.Value() + + cols = append(cols, "keep_pronouns_private") + } + if opts.AllowGitHook.Has() { u.AllowGitHook = opts.AllowGitHook.Value() diff --git a/templates/shared/user/profile_big_avatar.tmpl b/templates/shared/user/profile_big_avatar.tmpl index c09fed5f1e..d718fa390f 100644 --- a/templates/shared/user/profile_big_avatar.tmpl +++ b/templates/shared/user/profile_big_avatar.tmpl @@ -16,7 +16,7 @@
{{if .ContextUser.FullName}}{{.ContextUser.FullName}}{{end}} - {{.ContextUser.Name}}{{if .ContextUser.Pronouns}} · {{.ContextUser.Pronouns}}{{end}} {{if .IsAdmin}} + {{.ContextUser.Name}} {{if .ContextUser.GetPronouns .IsSigned}} · {{.ContextUser.GetPronouns .IsSigned}}{{end}} {{if .IsAdmin}} {{svg "octicon-gear" 18}} diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index c7cb257e21..cfe6ae7656 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -27954,6 +27954,10 @@ "type": "boolean", "x-go-name": "HideEmail" }, + "hide_pronouns": { + "type": "boolean", + "x-go-name": "HidePronouns" + }, "language": { "type": "string", "x-go-name": "Language" @@ -28006,6 +28010,10 @@ "type": "boolean", "x-go-name": "HideEmail" }, + "hide_pronouns": { + "type": "boolean", + "x-go-name": "HidePronouns" + }, "language": { "type": "string", "x-go-name": "Language" diff --git a/templates/user/settings/profile.tmpl b/templates/user/settings/profile.tmpl index 4cc30d66cf..503a977158 100644 --- a/templates/user/settings/profile.tmpl +++ b/templates/user/settings/profile.tmpl @@ -120,6 +120,12 @@ {{ctx.Locale.Tr "settings.keep_activity_private"}} {{ctx.Locale.Tr "settings.keep_activity_private.description" (printf "/%s?tab=activity" .SignedUser.Name)}} + + diff --git a/tests/integration/user_test.go b/tests/integration/user_test.go index d2b5f112a3..3da8a3ae1d 100644 --- a/tests/integration/user_test.go +++ b/tests/integration/user_test.go @@ -438,8 +438,16 @@ func TestUserHints(t *testing.T) { func TestUserPronouns(t *testing.T) { defer tests.PrepareTestEnv(t)() - session := loginUser(t, "user2") - token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteUser) + // user1 is admin, using user2 and user10 respectively instead. + // This is explicitly mentioned here because of the unconventional + // variable naming scheme. + firstUserSession := loginUser(t, "user2") + firstUserToken := getTokenForLoggedInUser(t, firstUserSession, auth_model.AccessTokenScopeWriteUser) + + // This user has the HidePronouns setting enabled. + // Check the fixture! + secondUserSession := loginUser(t, "user10") + secondUserToken := getTokenForLoggedInUser(t, secondUserSession, auth_model.AccessTokenScopeWriteUser) adminUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{IsAdmin: true}) adminSession := loginUser(t, adminUser.Name) @@ -449,8 +457,10 @@ func TestUserPronouns(t *testing.T) { t.Run("user", func(t *testing.T) { defer tests.PrintCurrentTest(t)() - req := NewRequest(t, "GET", "/api/v1/user").AddTokenAuth(token) - resp := MakeRequest(t, req, http.StatusOK) + // secondUserToken was chosen arbitrarily and should have no impact. + // See next comment. + req := NewRequest(t, "GET", "/api/v1/user").AddTokenAuth(secondUserToken) + resp := firstUserSession.MakeRequest(t, req, http.StatusOK) // We check the raw JSON, because we want to test the response, not // what it decodes into. Contents doesn't matter, we're testing the @@ -468,16 +478,22 @@ func TestUserPronouns(t *testing.T) { // what it decodes into. Contents doesn't matter, we're testing the // presence only. assert.Contains(t, resp.Body.String(), `"pronouns":`) + + req = NewRequest(t, "GET", "/api/v1/users/user10") + resp = MakeRequest(t, req, http.StatusOK) + + // Same deal here. + assert.Contains(t, resp.Body.String(), `"pronouns":`) }) t.Run("user/settings", func(t *testing.T) { defer tests.PrintCurrentTest(t)() - // Set pronouns first + // Set pronouns first for user2 pronouns := "they/them" req := NewRequestWithJSON(t, "PATCH", "/api/v1/user/settings", &api.UserSettingsOptions{ Pronouns: &pronouns, - }).AddTokenAuth(token) + }).AddTokenAuth(firstUserToken) resp := MakeRequest(t, req, http.StatusOK) // Verify the response @@ -486,7 +502,7 @@ func TestUserPronouns(t *testing.T) { assert.Equal(t, pronouns, user.Pronouns) // Verify retrieving the settings again - req = NewRequest(t, "GET", "/api/v1/user/settings").AddTokenAuth(token) + req = NewRequest(t, "GET", "/api/v1/user/settings").AddTokenAuth(firstUserToken) resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &user) @@ -497,22 +513,40 @@ func TestUserPronouns(t *testing.T) { defer tests.PrintCurrentTest(t)() // Set the pronouns for user2 - pronouns := "she/her" + pronouns := "he/him" req := NewRequestWithJSON(t, "PATCH", "/api/v1/admin/users/user2", &api.EditUserOption{ Pronouns: &pronouns, }).AddTokenAuth(adminToken) resp := MakeRequest(t, req, http.StatusOK) // Verify the API response - var user *api.User - DecodeJSON(t, resp, &user) - assert.Equal(t, pronouns, user.Pronouns) + var user2 *api.User + DecodeJSON(t, resp, &user2) + assert.Equal(t, pronouns, user2.Pronouns) - // Verify via user2 too - req = NewRequest(t, "GET", "/api/v1/user").AddTokenAuth(token) + // Verify via user2 + req = NewRequest(t, "GET", "/api/v1/user").AddTokenAuth(firstUserToken) resp = MakeRequest(t, req, http.StatusOK) - DecodeJSON(t, resp, &user) - assert.Equal(t, pronouns, user.Pronouns) + DecodeJSON(t, resp, &user2) + assert.Equal(t, pronouns, user2.Pronouns) // TODO: This fails for some reason + + // Set the pronouns for user10 + pronouns = "he/him" + req = NewRequestWithJSON(t, "PATCH", "/api/v1/admin/users/user10", &api.EditUserOption{ + Pronouns: &pronouns, + }).AddTokenAuth(adminToken) + resp = MakeRequest(t, req, http.StatusOK) + + // Verify the API response + var user10 *api.User + DecodeJSON(t, resp, &user10) + assert.Equal(t, pronouns, user10.Pronouns) + + // Verify via user10 + req = NewRequest(t, "GET", "/api/v1/user").AddTokenAuth(secondUserToken) + resp = MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &user10) + assert.Equal(t, pronouns, user10.Pronouns) }) }) @@ -520,10 +554,10 @@ func TestUserPronouns(t *testing.T) { defer tests.PrintCurrentTest(t)() // Set the pronouns to a known state via the API - pronouns := "she/her" + pronouns := "they/them" req := NewRequestWithJSON(t, "PATCH", "/api/v1/user/settings", &api.UserSettingsOptions{ Pronouns: &pronouns, - }).AddTokenAuth(token) + }).AddTokenAuth(firstUserToken) MakeRequest(t, req, http.StatusOK) t.Run("profile view", func(t *testing.T) { @@ -534,14 +568,14 @@ func TestUserPronouns(t *testing.T) { htmlDoc := NewHTMLParser(t, resp.Body) userNameAndPronouns := strings.TrimSpace(htmlDoc.Find(".profile-avatar-name .username").Text()) - assert.Contains(t, userNameAndPronouns, pronouns) + assert.NotContains(t, userNameAndPronouns, pronouns) }) t.Run("settings", func(t *testing.T) { defer tests.PrintCurrentTest(t)() req := NewRequest(t, "GET", "/user/settings") - resp := session.MakeRequest(t, req, http.StatusOK) + resp := firstUserSession.MakeRequest(t, req, http.StatusOK) htmlDoc := NewHTMLParser(t, resp.Body) // Check that the field is present @@ -550,12 +584,12 @@ func TestUserPronouns(t *testing.T) { assert.Equal(t, pronouns, pronounField) // Check that updating the field works - newPronouns := "they/them" + newPronouns := "she/her" req = NewRequestWithValues(t, "POST", "/user/settings", map[string]string{ - "_csrf": GetCSRF(t, session, "/user/settings"), + "_csrf": GetCSRF(t, firstUserSession, "/user/settings"), "pronouns": newPronouns, }) - session.MakeRequest(t, req, http.StatusSeeOther) + firstUserSession.MakeRequest(t, req, http.StatusSeeOther) user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user2"}) assert.Equal(t, newPronouns, user2.Pronouns)