From 4fdd2de1bb08049fbaa0902fa06e06327572a41f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 10 Jul 2025 09:12:12 -0700 Subject: [PATCH 1/3] Fix bug when updating user visibility to public --- modules/optional/option.go | 14 ++++++++++---- routers/api/v1/admin/user.go | 16 +++++++++------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/modules/optional/option.go b/modules/optional/option.go index 6075c6347e098..5524b375d967d 100644 --- a/modules/optional/option.go +++ b/modules/optional/option.go @@ -28,12 +28,18 @@ func FromPtr[T any](v *T) Option[T] { return Some(*v) } -func FromNonDefault[T comparable](v T) Option[T] { - var zero T - if v == zero { +func FromNonDefaultFunc[T comparable](f T, isZero func(T) bool) Option[T] { + if isZero(f) { return None[T]() } - return Some(v) + return Some(f) +} + +func FromNonDefault[T comparable](v T) Option[T] { + return FromNonDefaultFunc(v, func(v T) bool { + var zero T + return v == zero + }) } func (o Option[T]) Has() bool { diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index 8a267cc418d7f..d689cdcb6dbca 100644 --- a/routers/api/v1/admin/user.go +++ b/routers/api/v1/admin/user.go @@ -234,13 +234,15 @@ func EditUser(ctx *context.APIContext) { } opts := &user_service.UpdateOptions{ - FullName: optional.FromPtr(form.FullName), - Website: optional.FromPtr(form.Website), - Location: optional.FromPtr(form.Location), - Description: optional.FromPtr(form.Description), - IsActive: optional.FromPtr(form.Active), - IsAdmin: user_service.UpdateOptionFieldFromPtr(form.Admin), - Visibility: optional.FromNonDefault(api.VisibilityModes[form.Visibility]), + FullName: optional.FromPtr(form.FullName), + Website: optional.FromPtr(form.Website), + Location: optional.FromPtr(form.Location), + Description: optional.FromPtr(form.Description), + IsActive: optional.FromPtr(form.Active), + IsAdmin: user_service.UpdateOptionFieldFromPtr(form.Admin), + Visibility: optional.FromNonDefaultFunc(api.VisibilityModes[form.Visibility], func(v api.VisibleType) bool { + return v < api.VisibleTypePublic || v > api.VisibleTypePrivate + }), AllowGitHook: optional.FromPtr(form.AllowGitHook), AllowImportLocal: optional.FromPtr(form.AllowImportLocal), MaxRepoCreation: optional.FromPtr(form.MaxRepoCreation), From d87aa947baa247bae145dd64c436b23a3a39352a Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 10 Jul 2025 09:16:27 -0700 Subject: [PATCH 2/3] Add test and also fix org api --- modules/optional/option_test.go | 5 +++++ routers/api/v1/org/org.go | 12 +++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/modules/optional/option_test.go b/modules/optional/option_test.go index f600ff5a2c727..36bcf0c7457c3 100644 --- a/modules/optional/option_test.go +++ b/modules/optional/option_test.go @@ -56,6 +56,11 @@ func TestOption(t *testing.T) { opt3 := optional.FromNonDefault(1) assert.True(t, opt3.Has()) assert.Equal(t, int(1), opt3.Value()) + + opt4 := optional.FromNonDefaultFunc(1, func(t int) bool { + return t == 1 + }) + assert.False(t, opt4.Has()) } func Test_ParseBool(t *testing.T) { diff --git a/routers/api/v1/org/org.go b/routers/api/v1/org/org.go index 05744ba1552c8..d7656d4a92712 100644 --- a/routers/api/v1/org/org.go +++ b/routers/api/v1/org/org.go @@ -387,11 +387,13 @@ func Edit(ctx *context.APIContext) { } opts := &user_service.UpdateOptions{ - FullName: optional.Some(form.FullName), - Description: optional.Some(form.Description), - Website: optional.Some(form.Website), - Location: optional.Some(form.Location), - Visibility: optional.FromNonDefault(api.VisibilityModes[form.Visibility]), + FullName: optional.Some(form.FullName), + Description: optional.Some(form.Description), + Website: optional.Some(form.Website), + Location: optional.Some(form.Location), + Visibility: optional.FromNonDefaultFunc(api.VisibilityModes[form.Visibility], func(v api.VisibleType) bool { + return v < api.VisibleTypePublic || v > api.VisibleTypePrivate + }), RepoAdminChangeTeamAccess: optional.FromPtr(form.RepoAdminChangeTeamAccess), } if err := user_service.UpdateUser(ctx, ctx.Org.Organization.AsUser(), opts); err != nil { From df080cf2f38b077929e1ca2613adb87e5df88f48 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 11 Jul 2025 00:42:56 +0800 Subject: [PATCH 3/3] fix --- modules/optional/option.go | 17 +++++++++-------- modules/optional/option_test.go | 7 ++++--- routers/api/v1/admin/user.go | 16 +++++++--------- routers/api/v1/org/org.go | 12 +++++------- 4 files changed, 25 insertions(+), 27 deletions(-) diff --git a/modules/optional/option.go b/modules/optional/option.go index 5524b375d967d..cbecf86987347 100644 --- a/modules/optional/option.go +++ b/modules/optional/option.go @@ -28,18 +28,19 @@ func FromPtr[T any](v *T) Option[T] { return Some(*v) } -func FromNonDefaultFunc[T comparable](f T, isZero func(T) bool) Option[T] { - if isZero(f) { - return None[T]() +func FromMapLookup[K comparable, V any](m map[K]V, k K) Option[V] { + if v, ok := m[k]; ok { + return Some(v) } - return Some(f) + return None[V]() } func FromNonDefault[T comparable](v T) Option[T] { - return FromNonDefaultFunc(v, func(v T) bool { - var zero T - return v == zero - }) + var zero T + if v == zero { + return None[T]() + } + return Some(v) } func (o Option[T]) Has() bool { diff --git a/modules/optional/option_test.go b/modules/optional/option_test.go index 36bcf0c7457c3..ea80a2e3cb478 100644 --- a/modules/optional/option_test.go +++ b/modules/optional/option_test.go @@ -57,9 +57,10 @@ func TestOption(t *testing.T) { assert.True(t, opt3.Has()) assert.Equal(t, int(1), opt3.Value()) - opt4 := optional.FromNonDefaultFunc(1, func(t int) bool { - return t == 1 - }) + opt4 := optional.FromMapLookup(map[string]int{"a": 1}, "a") + assert.True(t, opt4.Has()) + assert.Equal(t, 1, opt4.Value()) + opt4 = optional.FromMapLookup(map[string]int{"a": 1}, "b") assert.False(t, opt4.Has()) } diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index d689cdcb6dbca..494bace585189 100644 --- a/routers/api/v1/admin/user.go +++ b/routers/api/v1/admin/user.go @@ -234,15 +234,13 @@ func EditUser(ctx *context.APIContext) { } opts := &user_service.UpdateOptions{ - FullName: optional.FromPtr(form.FullName), - Website: optional.FromPtr(form.Website), - Location: optional.FromPtr(form.Location), - Description: optional.FromPtr(form.Description), - IsActive: optional.FromPtr(form.Active), - IsAdmin: user_service.UpdateOptionFieldFromPtr(form.Admin), - Visibility: optional.FromNonDefaultFunc(api.VisibilityModes[form.Visibility], func(v api.VisibleType) bool { - return v < api.VisibleTypePublic || v > api.VisibleTypePrivate - }), + FullName: optional.FromPtr(form.FullName), + Website: optional.FromPtr(form.Website), + Location: optional.FromPtr(form.Location), + Description: optional.FromPtr(form.Description), + IsActive: optional.FromPtr(form.Active), + IsAdmin: user_service.UpdateOptionFieldFromPtr(form.Admin), + Visibility: optional.FromMapLookup(api.VisibilityModes, form.Visibility), AllowGitHook: optional.FromPtr(form.AllowGitHook), AllowImportLocal: optional.FromPtr(form.AllowImportLocal), MaxRepoCreation: optional.FromPtr(form.MaxRepoCreation), diff --git a/routers/api/v1/org/org.go b/routers/api/v1/org/org.go index d7656d4a92712..cd676860658dc 100644 --- a/routers/api/v1/org/org.go +++ b/routers/api/v1/org/org.go @@ -387,13 +387,11 @@ func Edit(ctx *context.APIContext) { } opts := &user_service.UpdateOptions{ - FullName: optional.Some(form.FullName), - Description: optional.Some(form.Description), - Website: optional.Some(form.Website), - Location: optional.Some(form.Location), - Visibility: optional.FromNonDefaultFunc(api.VisibilityModes[form.Visibility], func(v api.VisibleType) bool { - return v < api.VisibleTypePublic || v > api.VisibleTypePrivate - }), + FullName: optional.Some(form.FullName), + Description: optional.Some(form.Description), + Website: optional.Some(form.Website), + Location: optional.Some(form.Location), + Visibility: optional.FromMapLookup(api.VisibilityModes, form.Visibility), RepoAdminChangeTeamAccess: optional.FromPtr(form.RepoAdminChangeTeamAccess), } if err := user_service.UpdateUser(ctx, ctx.Org.Organization.AsUser(), opts); err != nil {