From 76ca258f2392d194bed31d3b6cb29ba98077b49e Mon Sep 17 00:00:00 2001 From: Johnny Date: Mon, 11 Aug 2025 23:19:56 +0800 Subject: [PATCH] chore: simplify update user settings --- server/router/api/v1/user_service.go | 110 ++++++---------- server/router/api/v1/user_service_test.go | 146 ---------------------- 2 files changed, 38 insertions(+), 218 deletions(-) delete mode 100644 server/router/api/v1/user_service_test.go diff --git a/server/router/api/v1/user_service.go b/server/router/api/v1/user_service.go index 14b802a9d..4e27c3cd6 100644 --- a/server/router/api/v1/user_service.go +++ b/server/router/api/v1/user_service.go @@ -385,11 +385,46 @@ func (s *APIV1Service) UpdateUserSetting(ctx context.Context, request *v1pb.Upda return nil, status.Errorf(codes.NotFound, "%s not found", storeKey.String()) } - // merge only the fields specified by UpdateMask - merged := mergeUserSettingWithMask(existingUserSetting, request.Setting, storeKey, request.UpdateMask.Paths) + // Only GENERAL settings are supported via UpdateUserSetting + // Other setting types have dedicated service methods + if storeKey != storepb.UserSetting_GENERAL { + return nil, status.Errorf(codes.InvalidArgument, "setting type %s should not be updated via UpdateUserSetting", storeKey.String()) + } + + // Start with existing general setting values + existingGeneral := existingUserSetting.GetGeneral() + updatedGeneral := &v1pb.UserSetting_GeneralSetting{ + Appearance: existingGeneral.GetAppearance(), + MemoVisibility: existingGeneral.GetMemoVisibility(), + Locale: existingGeneral.GetLocale(), + Theme: existingGeneral.GetTheme(), + } + + // Apply updates for fields specified in the update mask + incomingGeneral := request.Setting.GetGeneralSetting() + for _, field := range request.UpdateMask.Paths { + switch field { + case "appearance": + updatedGeneral.Appearance = incomingGeneral.Appearance + case "memoVisibility": + updatedGeneral.MemoVisibility = incomingGeneral.MemoVisibility + case "theme": + updatedGeneral.Theme = incomingGeneral.Theme + case "locale": + updatedGeneral.Locale = incomingGeneral.Locale + } + } + + // Create the updated setting + updatedSetting := &v1pb.UserSetting{ + Name: request.Setting.Name, + Value: &v1pb.UserSetting_GeneralSetting_{ + GeneralSetting: updatedGeneral, + }, + } // Convert API setting to store setting - storeSetting, err := convertUserSettingToStore(merged, userID, storeKey) + storeSetting, err := convertUserSettingToStore(updatedSetting, userID, storeKey) if err != nil { return nil, status.Errorf(codes.InvalidArgument, "failed to convert setting: %v", err) } @@ -1335,72 +1370,3 @@ func (s *APIV1Service) validateUserFilter(_ context.Context, filterStr string) e } return nil } - -func mergeUserSettingWithMask(existing *storepb.UserSetting, incoming *v1pb.UserSetting, key storepb.UserSetting_Key, paths []string) *v1pb.UserSetting { - if incoming == nil { - return &v1pb.UserSetting{} - } - - switch key { - case storepb.UserSetting_GENERAL: - var gs *v1pb.UserSetting_GeneralSetting - - if existing == nil { - gs = &v1pb.UserSetting_GeneralSetting{ - Locale: "en", - Appearance: "system", - MemoVisibility: "PRIVATE", - Theme: "", - } - } else { - gs = &v1pb.UserSetting_GeneralSetting{ - Appearance: existing.GetGeneral().GetAppearance(), - MemoVisibility: existing.GetGeneral().GetMemoVisibility(), - Locale: existing.GetGeneral().GetLocale(), - Theme: existing.GetGeneral().GetTheme(), - } - } - - for _, field := range paths { - switch field { - case "appearance": - gs.Appearance = incoming.GetGeneralSetting().Appearance - case "memoVisibility": - gs.MemoVisibility = incoming.GetGeneralSetting().MemoVisibility - case "theme": - gs.Theme = incoming.GetGeneralSetting().Theme - case "locale": - gs.Locale = incoming.GetGeneralSetting().Locale - } - } - - return &v1pb.UserSetting{ - Name: incoming.Name, - Value: &v1pb.UserSetting_GeneralSetting_{ - GeneralSetting: gs, - }, - } - - case storepb.UserSetting_SHORTCUTS: - // handled by the FE calling shortcut_service.CreateShortcut - // if the FE wants to modify shortcuts by calling the user_service we need to handle below - - return incoming - case storepb.UserSetting_WEBHOOKS: - // handled by the FE calling user_service.CreateUserWebhook - // if the FE wants to modify webhooks by calling the user_service we need to handle below - - return incoming - case storepb.UserSetting_ACCESS_TOKENS: - // handled by the FE calling user_service.CreateUserAccessToken - // if the FE wants to modify access tokens by calling the user_service we need to handle below - - return incoming - case storepb.UserSetting_SESSIONS: - // handled by the FE calling auth_service.CreateSession - // if the FE wants to modify sessions by calling the user_service we need to handle below - return incoming - default: - return incoming - } -} diff --git a/server/router/api/v1/user_service_test.go b/server/router/api/v1/user_service_test.go deleted file mode 100644 index ae5348281..000000000 --- a/server/router/api/v1/user_service_test.go +++ /dev/null @@ -1,146 +0,0 @@ -package v1 - -import ( - "reflect" - "testing" - - v1pb "github.com/usememos/memos/proto/gen/api/v1" - storepb "github.com/usememos/memos/proto/gen/store" -) - -func TestMergeUserSettingWithMask(t *testing.T) { - tests := []struct { - name string - existing *storepb.UserSetting - incoming *v1pb.UserSetting - key storepb.UserSetting_Key - paths []string - expected *v1pb.UserSetting - }{ - { - name: "adds new field without removing existing fields", - existing: &storepb.UserSetting{ - UserId: 1, - Key: storepb.UserSetting_GENERAL, - Value: &storepb.UserSetting_General{ - General: &storepb.GeneralUserSetting{ - MemoVisibility: "PROTECTED", - }, - }, - }, - incoming: &v1pb.UserSetting{ - Value: &v1pb.UserSetting_GeneralSetting_{ - GeneralSetting: &v1pb.UserSetting_GeneralSetting{ - Appearance: "light", - }, - }, - }, - key: storepb.UserSetting_GENERAL, - paths: []string{"appearance"}, - expected: &v1pb.UserSetting{ - Value: &v1pb.UserSetting_GeneralSetting_{ - GeneralSetting: &v1pb.UserSetting_GeneralSetting{ - Appearance: "light", - MemoVisibility: "PROTECTED", - }, - }, - }, - }, - { - name: "adds new field when no existing fields exist", - existing: &storepb.UserSetting{ - UserId: 1, - Key: storepb.UserSetting_GENERAL, - Value: &storepb.UserSetting_General{}, - }, - incoming: &v1pb.UserSetting{ - Value: &v1pb.UserSetting_GeneralSetting_{ - GeneralSetting: &v1pb.UserSetting_GeneralSetting{ - Theme: "whitewall", - }, - }, - }, - key: storepb.UserSetting_GENERAL, - paths: []string{"theme"}, - expected: &v1pb.UserSetting{ - Value: &v1pb.UserSetting_GeneralSetting_{ - GeneralSetting: &v1pb.UserSetting_GeneralSetting{ - Theme: "whitewall", - }, - }, - }, - }, - { - name: "updates existing field without removing existing fields", - existing: &storepb.UserSetting{ - UserId: 1, - Key: storepb.UserSetting_GENERAL, - Value: &storepb.UserSetting_General{ - General: &storepb.GeneralUserSetting{ - Appearance: "dark", - MemoVisibility: "PUBLIC", - }, - }, - }, - incoming: &v1pb.UserSetting{ - Value: &v1pb.UserSetting_GeneralSetting_{ - GeneralSetting: &v1pb.UserSetting_GeneralSetting{ - Appearance: "light", - }, - }, - }, - key: storepb.UserSetting_GENERAL, - paths: []string{"appearance"}, - expected: &v1pb.UserSetting{ - Value: &v1pb.UserSetting_GeneralSetting_{ - GeneralSetting: &v1pb.UserSetting_GeneralSetting{ - Appearance: "light", - MemoVisibility: "PUBLIC", - }, - }, - }, - }, - { - name: "updates multiple fields without removing existing fields", - existing: &storepb.UserSetting{ - UserId: 1, - Key: storepb.UserSetting_GENERAL, - Value: &storepb.UserSetting_General{ - General: &storepb.GeneralUserSetting{ - Appearance: "system", - }, - }, - }, - incoming: &v1pb.UserSetting{ - Value: &v1pb.UserSetting_GeneralSetting_{ - GeneralSetting: &v1pb.UserSetting_GeneralSetting{ - Appearance: "dark", - Theme: "paper", - MemoVisibility: "PROTECTED", - }, - }, - }, - key: storepb.UserSetting_GENERAL, - paths: []string{"theme", "memoVisibility", "appearance"}, - expected: &v1pb.UserSetting{ - Value: &v1pb.UserSetting_GeneralSetting_{ - GeneralSetting: &v1pb.UserSetting_GeneralSetting{ - Appearance: "dark", - MemoVisibility: "PROTECTED", - Theme: "paper", - }, - }, - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - actual := mergeUserSettingWithMask(tt.existing, tt.incoming, tt.key, tt.paths) - - if !reflect.DeepEqual(actual, tt.expected) { - t.Errorf("expected %v but got %v", tt.expected, actual) - } - }) - } -}