fix: keep group avatar update for legacy groups
This commit is contained in:
@@ -4680,13 +4680,19 @@ func serveIMGroupDetail(wrt http.ResponseWriter, req *http.Request, topic string
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
patch := types.GroupSettingsPatch{Name: body.Name, AvatarFileID: body.AvatarFileID, Description: body.Description, Version: body.Version}
|
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 _, err = store.GroupBusiness.UpdateSettings(principal.TenantID, topic, patch); err != nil {
|
||||||
|
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 {
|
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)
|
logs.Warn.Printf("group[%s]: failed to roll back topic profile after business update error: %v", topic, rollbackErr)
|
||||||
}
|
}
|
||||||
writeIMRelationError(wrt, req, err)
|
writeIMRelationError(wrt, req, err)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
}
|
||||||
dispatchIMGroupEvents(principal.TenantID)
|
dispatchIMGroupEvents(principal.TenantID)
|
||||||
updated, err := store.Topics.Get(principal.TenantID, topic)
|
updated, err := store.Topics.Get(principal.TenantID, topic)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -4694,9 +4700,11 @@ func serveIMGroupDetail(wrt http.ResponseWriter, req *http.Request, topic string
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
payload := groupDetailPayload(updated, selfSub)
|
payload := groupDetailPayload(updated, selfSub)
|
||||||
|
if settingsUpdated {
|
||||||
if settings, err := store.GroupBusiness.GetSettings(principal.TenantID, topic); err == nil {
|
if settings, err := store.GroupBusiness.GetSettings(principal.TenantID, topic); err == nil {
|
||||||
payload = mergeGroupSettingsPayload(payload, settings)
|
payload = mergeGroupSettingsPayload(payload, settings)
|
||||||
}
|
}
|
||||||
|
}
|
||||||
writeIMSuccess(wrt, req, payload)
|
writeIMSuccess(wrt, req, payload)
|
||||||
default:
|
default:
|
||||||
writeIMError(wrt, req, http.StatusMethodNotAllowed, 30002, imReasonMethodNotAllowed, "method not allowed", nil)
|
writeIMError(wrt, req, http.StatusMethodNotAllowed, 30002, imReasonMethodNotAllowed, "method not allowed", nil)
|
||||||
|
|||||||
@@ -335,6 +335,7 @@ type testGroupBusinessStore struct {
|
|||||||
updatedSettings types.GroupSettingsPatch
|
updatedSettings types.GroupSettingsPatch
|
||||||
updatedMembers []types.GroupMemberPatch
|
updatedMembers []types.GroupMemberPatch
|
||||||
updatedRoleMode types.AccessMode
|
updatedRoleMode types.AccessMode
|
||||||
|
updateSettingsErr error
|
||||||
updateMemberErr error
|
updateMemberErr error
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -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) {
|
func (s *testGroupBusinessStore) UpdateSettings(_ types.TenantID, topic string, patch types.GroupSettingsPatch) (*types.GroupSettings, error) {
|
||||||
s.updatedSettings = patch
|
s.updatedSettings = patch
|
||||||
|
if s.updateSettingsErr != nil {
|
||||||
|
return nil, s.updateSettingsErr
|
||||||
|
}
|
||||||
settings, _ := s.GetSettings(0, topic)
|
settings, _ := s.GetSettings(0, topic)
|
||||||
if patch.Name != nil {
|
if patch.Name != nil {
|
||||||
settings.Name = *patch.Name
|
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) {
|
func TestServeIMGroupMyProfileKeepsSubscriptionNicknameWhenBusinessMemberMissing(t *testing.T) {
|
||||||
ctrl := gomock.NewController(t)
|
ctrl := gomock.NewController(t)
|
||||||
defer ctrl.Finish()
|
defer ctrl.Finish()
|
||||||
|
|||||||
Reference in New Issue
Block a user