diff --git a/server/im_http.go b/server/im_http.go index b4f71b2..72662a0 100644 --- a/server/im_http.go +++ b/server/im_http.go @@ -4680,12 +4680,18 @@ func serveIMGroupDetail(wrt http.ResponseWriter, req *http.Request, topic string return } patch := types.GroupSettingsPatch{Name: body.Name, AvatarFileID: body.AvatarFileID, Description: body.Description, Version: body.Version} + settingsUpdated := true if _, err = store.GroupBusiness.UpdateSettings(principal.TenantID, topic, patch); err != nil { - if rollbackErr := store.Topics.Update(principal.TenantID, topic, map[string]any{"Public": group.Public}); rollbackErr != nil { - logs.Warn.Printf("group[%s]: failed to roll back topic profile after business update error: %v", topic, rollbackErr) + if errors.Is(err, types.ErrNotFound) { + settingsUpdated = false + logs.Warn.Printf("group[%s]: business settings missing while patching group profile; kept topic public update", topic) + } else { + if rollbackErr := store.Topics.Update(principal.TenantID, topic, map[string]any{"Public": group.Public}); rollbackErr != nil { + logs.Warn.Printf("group[%s]: failed to roll back topic profile after business update error: %v", topic, rollbackErr) + } + writeIMRelationError(wrt, req, err) + return } - writeIMRelationError(wrt, req, err) - return } dispatchIMGroupEvents(principal.TenantID) updated, err := store.Topics.Get(principal.TenantID, topic) @@ -4694,8 +4700,10 @@ func serveIMGroupDetail(wrt http.ResponseWriter, req *http.Request, topic string return } payload := groupDetailPayload(updated, selfSub) - if settings, err := store.GroupBusiness.GetSettings(principal.TenantID, topic); err == nil { - payload = mergeGroupSettingsPayload(payload, settings) + if settingsUpdated { + if settings, err := store.GroupBusiness.GetSettings(principal.TenantID, topic); err == nil { + payload = mergeGroupSettingsPayload(payload, settings) + } } writeIMSuccess(wrt, req, payload) default: diff --git a/server/im_http_test.go b/server/im_http_test.go index 34b6a5d..7a6143d 100644 --- a/server/im_http_test.go +++ b/server/im_http_test.go @@ -325,17 +325,18 @@ type testSafetyStore struct { } type testGroupBusinessStore struct { - settings *types.GroupSettings - member *types.GroupMember - joinRequests []types.GroupJoinRequest - announcement *types.GroupAnnouncement - mutes []types.GroupMute - files []types.GroupFile - invitations []types.GroupInvitation - updatedSettings types.GroupSettingsPatch - updatedMembers []types.GroupMemberPatch - updatedRoleMode types.AccessMode - updateMemberErr error + settings *types.GroupSettings + member *types.GroupMember + joinRequests []types.GroupJoinRequest + announcement *types.GroupAnnouncement + mutes []types.GroupMute + files []types.GroupFile + invitations []types.GroupInvitation + updatedSettings types.GroupSettingsPatch + updatedMembers []types.GroupMemberPatch + updatedRoleMode types.AccessMode + updateSettingsErr error + updateMemberErr error } func (s *testGroupBusinessStore) CreateGroup(_ types.TenantID, _ *types.Topic, _ types.Uid, _ any, @@ -358,6 +359,9 @@ func (s *testGroupBusinessStore) GetSettings(_ types.TenantID, topic string) (*t func (s *testGroupBusinessStore) UpdateSettings(_ types.TenantID, topic string, patch types.GroupSettingsPatch) (*types.GroupSettings, error) { s.updatedSettings = patch + if s.updateSettingsErr != nil { + return nil, s.updateSettingsErr + } settings, _ := s.GetSettings(0, topic) if patch.Name != nil { settings.Name = *patch.Name @@ -3858,6 +3862,84 @@ func TestServeIMGroupDetailPatchSettingsProfileAndLeave(t *testing.T) { } } +func TestServeIMGroupDetailPatchKeepsTopicProfileWhenBusinessSettingsMissing(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + oldStore := store.Store + oldGroupBusiness := store.GroupBusiness + mockStore := mock_store.NewMockPersistentStorageInterface(ctrl) + tokenAuth := mock_auth.NewMockAuthHandler(ctrl) + mockStore.EXPECT().GetLogicalAuthHandler("token").Return(tokenAuth) + tokenAuth.EXPECT().IsInitialized().Return(true) + tokenAuth.EXPECT().Authenticate(auth.AuthContext{}, []byte("token-1")). + Return(&auth.Rec{TenantID: 7, Uid: 42, AuthLevel: auth.LevelAuth}, nil, nil) + store.Store = mockStore + oldTopics := store.Topics + oldSubs := store.Subs + topics := mock_store.NewMockTopicsPersistenceInterface(ctrl) + subs := mock_store.NewMockSubsPersistenceInterface(ctrl) + store.Topics = topics + store.Subs = subs + store.GroupBusiness = &testGroupBusinessStore{updateSettingsErr: types.ErrNotFound} + defer func() { + store.Store = oldStore + store.Topics = oldTopics + store.Subs = oldSubs + store.GroupBusiness = oldGroupBusiness + }() + selfSub := &types.Subscription{ + TenantID: 7, + User: types.Uid(42).String(), + Topic: "grpAlpha", + ModeWant: types.ModeCFull, + ModeGiven: types.ModeCFull, + Private: map[string]any{"version": 1, "profile_version": 1}, + } + group := &types.Topic{ + ObjHeader: types.ObjHeader{Id: "grpAlpha"}, + TenantID: 7, + Owner: types.Uid(42).String(), + SubCnt: 3, + Public: map[string]any{"name": "Alpha", "version": 1}, + } + var savedPublic map[string]any + subs.EXPECT().Get(types.TenantID(7), "grpAlpha", types.Uid(42), false).Return(selfSub, nil) + topics.EXPECT().Get(types.TenantID(7), "grpAlpha").Return(group, nil) + topics.EXPECT().Update(types.TenantID(7), "grpAlpha", gomock.Any()).DoAndReturn( + func(_ types.TenantID, _ string, update map[string]any) error { + public, ok := update["Public"].(map[string]any) + if !ok { + t.Fatalf("Public update missing: %#v", update) + } + savedPublic = public + return nil + }, + ) + topics.EXPECT().Get(types.TenantID(7), "grpAlpha").DoAndReturn( + func(_ types.TenantID, _ string) (*types.Topic, error) { + return &types.Topic{ + ObjHeader: types.ObjHeader{Id: "grpAlpha"}, + TenantID: 7, + Owner: types.Uid(42).String(), + SubCnt: 3, + Public: savedPublic, + }, nil + }, + ) + + req := httptest.NewRequest(http.MethodPatch, "/v1/im/groups/grpAlpha", bytes.NewBufferString(`{"avatar_file_id":"file-avatar"}`)) + req.Header.Set("Authorization", "Bearer token-1") + rec := httptest.NewRecorder() + serveIMGroupResource(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("patch status: got %d body=%s", rec.Code, rec.Body.String()) + } + if data := decodeIMHTTPResponse(t, rec).Data.(map[string]any); data["avatar_file_id"] != "file-avatar" || data["version"].(float64) != 2 { + t.Fatalf("unexpected patch response: %#v", data) + } +} + func TestServeIMGroupMyProfileKeepsSubscriptionNicknameWhenBusinessMemberMissing(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish()