Changes method name, removes error case. (#11427)

* Changes method name, removes previous error case.

* Fixes test now that the method is idempotent.

api4/channel_test.go

@@ -2178,7 +2178,7 @@ func TestAddChannelMember(t *testing.T) {
 	require.Nil(t, appErr)
 
 	// Add user to group
-	_, appErr = th.App.CreateOrRestoreGroupMember(th.Group.Id, user.Id)
+	_, appErr = th.App.UpsertGroupMember(th.Group.Id, user.Id)
 	require.Nil(t, appErr)
 
 	_, resp = th.SystemAdminClient.AddChannelMember(privateChannel.Id, user.Id)
@@ -2748,9 +2748,9 @@ func TestChannelMembersMinusGroupMembers(t *testing.T) {
 	group1 := th.CreateGroup()
 	group2 := th.CreateGroup()
 
-	_, err = th.App.CreateOrRestoreGroupMember(group1.Id, user1.Id)
+	_, err = th.App.UpsertGroupMember(group1.Id, user1.Id)
 	require.Nil(t, err)
-	_, err = th.App.CreateOrRestoreGroupMember(group2.Id, user2.Id)
+	_, err = th.App.UpsertGroupMember(group2.Id, user2.Id)
 	require.Nil(t, err)
 
 	// No permissions

api4/team_test.go

@@ -1540,7 +1540,7 @@ func TestAddTeamMember(t *testing.T) {
 	require.Nil(t, err)
 
 	// Add user to group
-	_, err = th.App.CreateOrRestoreGroupMember(th.Group.Id, otherUser.Id)
+	_, err = th.App.UpsertGroupMember(th.Group.Id, otherUser.Id)
 	require.Nil(t, err)
 
 	_, resp = th.SystemAdminClient.AddTeamMember(team.Id, otherUser.Id)
@@ -1755,7 +1755,7 @@ func TestAddTeamMembers(t *testing.T) {
 	require.Nil(t, err)
 
 	// Add user to group
-	_, err = th.App.CreateOrRestoreGroupMember(th.Group.Id, userList[0])
+	_, err = th.App.UpsertGroupMember(th.Group.Id, userList[0])
 	require.Nil(t, err)
 
 	_, resp = Client.AddTeamMembers(team.Id, userList)
@@ -2525,9 +2525,9 @@ func TestTeamMembersMinusGroupMembers(t *testing.T) {
 	group1 := th.CreateGroup()
 	group2 := th.CreateGroup()
 
-	_, err = th.App.CreateOrRestoreGroupMember(group1.Id, user1.Id)
+	_, err = th.App.UpsertGroupMember(group1.Id, user1.Id)
 	require.Nil(t, err)
-	_, err = th.App.CreateOrRestoreGroupMember(group2.Id, user2.Id)
+	_, err = th.App.UpsertGroupMember(group2.Id, user2.Id)
 	require.Nil(t, err)
 
 	// No permissions

app/group.go

@@ -79,8 +79,8 @@ func (a *App) GetGroupMemberUsersPage(groupID string, page int, perPage int) ([]
 	return members, count, nil
 }
 
-func (a *App) CreateOrRestoreGroupMember(groupID string, userID string) (*model.GroupMember, *model.AppError) {
+func (a *App) UpsertGroupMember(groupID string, userID string) (*model.GroupMember, *model.AppError) {
-	result := <-a.Srv.Store.Group().CreateOrRestoreMember(groupID, userID)
+	result := <-a.Srv.Store.Group().UpsertMember(groupID, userID)
 	if result.Err != nil {
 		return nil, result.Err
 	}

app/group_test.go

@@ -105,20 +105,20 @@ func TestCreateOrRestoreGroupMember(t *testing.T) {
 	defer th.TearDown()
 	group := th.CreateGroup()
 
-	g, err := th.App.CreateOrRestoreGroupMember(group.Id, th.BasicUser.Id)
+	g, err := th.App.UpsertGroupMember(group.Id, th.BasicUser.Id)
 	require.Nil(t, err)
 	require.NotNil(t, g)
 
-	g, err = th.App.CreateOrRestoreGroupMember(group.Id, th.BasicUser.Id)
+	g, err = th.App.UpsertGroupMember(group.Id, th.BasicUser.Id)
-	require.NotNil(t, err)
+	require.Nil(t, err)
-	require.Nil(t, g)
+	require.NotNil(t, g)
 }
 
 func TestDeleteGroupMember(t *testing.T) {
 	th := Setup(t).InitBasic()
 	defer th.TearDown()
 	group := th.CreateGroup()
-	groupMember, err := th.App.CreateOrRestoreGroupMember(group.Id, th.BasicUser.Id)
+	groupMember, err := th.App.UpsertGroupMember(group.Id, th.BasicUser.Id)
 	require.Nil(t, err)
 	require.NotNil(t, groupMember)
 

app/syncables_test.go

@@ -89,12 +89,12 @@ func TestCreateDefaultMemberships(t *testing.T) {
 	singer1 := th.BasicUser
 	scientist1 := th.BasicUser2
 
-	_, err = th.App.CreateOrRestoreGroupMember(gleeGroup.Id, singer1.Id)
+	_, err = th.App.UpsertGroupMember(gleeGroup.Id, singer1.Id)
 	if err != nil {
 		t.Errorf("test groupmember not created: %s", err.Error())
 	}
 
-	scientistGroupMember, err := th.App.CreateOrRestoreGroupMember(scienceGroup.Id, scientist1.Id)
+	scientistGroupMember, err := th.App.UpsertGroupMember(scienceGroup.Id, scientist1.Id)
 	if err != nil {
 		t.Errorf("test groupmember not created: %s", err.Error())
 	}
@@ -375,7 +375,7 @@ func TestDeleteGroupMemberships(t *testing.T) {
 	require.Equal(t, 3, int(cmemberCount))
 
 	// add a user to the group
-	_, err = th.App.CreateOrRestoreGroupMember(group.Id, th.SystemAdminUser.Id)
+	_, err = th.App.UpsertGroupMember(group.Id, th.SystemAdminUser.Id)
 	require.Nil(t, err)
 
 	// run the delete

store/sqlstore/group_store.go

@@ -335,7 +335,7 @@ func (s *SqlGroupStore) GetMemberCount(groupID string) store.StoreChannel {
 	})
 }
 
-func (s *SqlGroupStore) CreateOrRestoreMember(groupID string, userID string) store.StoreChannel {
+func (s *SqlGroupStore) UpsertMember(groupID string, userID string) store.StoreChannel {
 	return store.Do(func(result *store.StoreResult) {
 
 		member := &model.GroupMember{
@@ -362,11 +362,6 @@ func (s *SqlGroupStore) CreateOrRestoreMember(groupID string, userID string) sto
 			}
 		}
 
-		if retrievedMember != nil && retrievedMember.DeleteAt == 0 {
-			result.Err = model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.sql_group.uniqueness_error", nil, "group_id="+member.GroupId+", user_id="+member.UserId, http.StatusBadRequest)
-			return
-		}
-
 		if retrievedMember == nil {
 			if err := s.GetMaster().Insert(member); err != nil {
 				if IsUniqueConstraintError(err, []string{"GroupId", "UserId", "groupmembers_pkey", "PRIMARY"}) {

store/store.go

@@ -583,7 +583,7 @@ type GroupStore interface {
 	GetMemberUsers(groupID string) StoreChannel
 	GetMemberUsersPage(groupID string, offset int, limit int) StoreChannel
 	GetMemberCount(groupID string) StoreChannel
-	CreateOrRestoreMember(groupID string, userID string) StoreChannel
+	UpsertMember(groupID string, userID string) StoreChannel
 	DeleteMember(groupID string, userID string) StoreChannel
 
 	CreateGroupSyncable(groupSyncable *model.GroupSyncable) (*model.GroupSyncable, *model.AppError)

store/storetest/group_store.go

@@ -26,7 +26,7 @@ func TestGroupStore(t *testing.T, ss store.Store) {
 
 	t.Run("GetMemberUsers", func(t *testing.T) { testGroupGetMemberUsers(t, ss) })
 	t.Run("GetMemberUsersPage", func(t *testing.T) { testGroupGetMemberUsersPage(t, ss) })
-	t.Run("CreateOrRestoreMember", func(t *testing.T) { testGroupCreateOrRestoreMember(t, ss) })
+	t.Run("UpsertMember", func(t *testing.T) { testGroupCreateOrRestoreMember(t, ss) })
 	t.Run("DeleteMember", func(t *testing.T) { testGroupDeleteMember(t, ss) })
 
 	t.Run("CreateGroupSyncable", func(t *testing.T) { testCreateGroupSyncable(t, ss) })
@@ -457,7 +457,7 @@ func testGroupGetMemberUsers(t *testing.T, ss store.Store) {
 	require.Nil(t, res.Err)
 	user1 := res.Data.(*model.User)
 
-	res = <-ss.Group().CreateOrRestoreMember(group.Id, user1.Id)
+	res = <-ss.Group().UpsertMember(group.Id, user1.Id)
 	require.Nil(t, res.Err)
 
 	u2 := &model.User{
@@ -468,7 +468,7 @@ func testGroupGetMemberUsers(t *testing.T, ss store.Store) {
 	require.Nil(t, res.Err)
 	user2 := res.Data.(*model.User)
 
-	res = <-ss.Group().CreateOrRestoreMember(group.Id, user2.Id)
+	res = <-ss.Group().UpsertMember(group.Id, user2.Id)
 	require.Nil(t, res.Err)
 
 	// Check returns members
@@ -511,7 +511,7 @@ func testGroupGetMemberUsersPage(t *testing.T, ss store.Store) {
 	require.Nil(t, res.Err)
 	user1 := res.Data.(*model.User)
 
-	res = <-ss.Group().CreateOrRestoreMember(group.Id, user1.Id)
+	res = <-ss.Group().UpsertMember(group.Id, user1.Id)
 	require.Nil(t, res.Err)
 
 	u2 := &model.User{
@@ -522,7 +522,7 @@ func testGroupGetMemberUsersPage(t *testing.T, ss store.Store) {
 	require.Nil(t, res.Err)
 	user2 := res.Data.(*model.User)
 
-	res = <-ss.Group().CreateOrRestoreMember(group.Id, user2.Id)
+	res = <-ss.Group().UpsertMember(group.Id, user2.Id)
 	require.Nil(t, res.Err)
 
 	// Check returns members
@@ -580,7 +580,7 @@ func testGroupCreateOrRestoreMember(t *testing.T, ss store.Store) {
 	user := res2.Data.(*model.User)
 
 	// Happy path
-	res3 := <-ss.Group().CreateOrRestoreMember(group.Id, user.Id)
+	res3 := <-ss.Group().UpsertMember(group.Id, user.Id)
 	require.Nil(t, res3.Err)
 	d2 := res3.Data.(*model.GroupMember)
 	require.Equal(t, d2.GroupId, group.Id)
@@ -589,16 +589,16 @@ func testGroupCreateOrRestoreMember(t *testing.T, ss store.Store) {
 	require.Zero(t, d2.DeleteAt)
 
 	// Duplicate composite key (GroupId, UserId)
-	res4 := <-ss.Group().CreateOrRestoreMember(group.Id, user.Id)
+	res4 := <-ss.Group().UpsertMember(group.Id, user.Id)
-	require.Equal(t, res4.Err.Id, "store.sql_group.uniqueness_error")
+	require.Nil(t, res4.Err)
 
 	// Invalid GroupId
-	res6 := <-ss.Group().CreateOrRestoreMember(model.NewId(), user.Id)
+	res6 := <-ss.Group().UpsertMember(model.NewId(), user.Id)
 	require.Equal(t, res6.Err.Id, "store.insert_error")
 
 	// Restores a deleted member
-	res := <-ss.Group().CreateOrRestoreMember(group.Id, user.Id)
+	res := <-ss.Group().UpsertMember(group.Id, user.Id)
-	require.NotNil(t, res.Err)
+	require.Nil(t, res.Err)
 
 	res = <-ss.Group().DeleteMember(group.Id, user.Id)
 	require.Nil(t, res.Err)
@@ -606,7 +606,7 @@ func testGroupCreateOrRestoreMember(t *testing.T, ss store.Store) {
 	res = <-ss.Group().GetMemberUsers(group.Id)
 	beforeRestoreCount := len(res.Data.([]*model.User))
 
-	res = <-ss.Group().CreateOrRestoreMember(group.Id, user.Id)
+	res = <-ss.Group().UpsertMember(group.Id, user.Id)
 	require.Nil(t, res.Err)
 
 	res = <-ss.Group().GetMemberUsers(group.Id)
@@ -637,7 +637,7 @@ func testGroupDeleteMember(t *testing.T, ss store.Store) {
 	user := res2.Data.(*model.User)
 
 	// Create member
-	res3 := <-ss.Group().CreateOrRestoreMember(group.Id, user.Id)
+	res3 := <-ss.Group().UpsertMember(group.Id, user.Id)
 	require.Nil(t, res3.Err)
 	d1 := res3.Data.(*model.GroupMember)
 
@@ -945,7 +945,7 @@ func testPendingAutoAddTeamMembers(t *testing.T, ss store.Store) {
 	user = res.Data.(*model.User)
 
 	// Create GroupMember
-	res = <-ss.Group().CreateOrRestoreMember(group.Id, user.Id)
+	res = <-ss.Group().UpsertMember(group.Id, user.Id)
 	require.Nil(t, res.Err)
 
 	// Create Team
@@ -981,7 +981,7 @@ func testPendingAutoAddTeamMembers(t *testing.T, ss store.Store) {
 	// Delete and restore GroupMember should return result
 	res = <-ss.Group().DeleteMember(group.Id, user.Id)
 	require.Nil(t, res.Err)
-	res = <-ss.Group().CreateOrRestoreMember(group.Id, user.Id)
+	res = <-ss.Group().UpsertMember(group.Id, user.Id)
 	require.Nil(t, res.Err)
 	teamMembers, err = ss.Group().TeamMembersToAdd(syncable.CreateAt + 1)
 	require.Nil(t, err)
@@ -1071,7 +1071,7 @@ func testPendingAutoAddTeamMembers(t *testing.T, ss store.Store) {
 	require.Len(t, teamMembers, 0)
 
 	// restore group member and verify
-	res = <-ss.Group().CreateOrRestoreMember(group.Id, user.Id)
+	res = <-ss.Group().UpsertMember(group.Id, user.Id)
 	teamMembers, err = ss.Group().TeamMembersToAdd(0)
 	require.Nil(t, err)
 	require.Len(t, teamMembers, 1)
@@ -1108,7 +1108,7 @@ func testPendingAutoAddChannelMembers(t *testing.T, ss store.Store) {
 	user = res.Data.(*model.User)
 
 	// Create GroupMember
-	res = <-ss.Group().CreateOrRestoreMember(group.Id, user.Id)
+	res = <-ss.Group().UpsertMember(group.Id, user.Id)
 	require.Nil(t, res.Err)
 
 	// Create Channel
@@ -1140,7 +1140,7 @@ func testPendingAutoAddChannelMembers(t *testing.T, ss store.Store) {
 	// Delete and restore GroupMember should return result
 	res = <-ss.Group().DeleteMember(group.Id, user.Id)
 	require.Nil(t, res.Err)
-	res = <-ss.Group().CreateOrRestoreMember(group.Id, user.Id)
+	res = <-ss.Group().UpsertMember(group.Id, user.Id)
 	require.Nil(t, res.Err)
 	channelMembers, err = ss.Group().ChannelMembersToAdd(syncable.CreateAt + 1)
 	require.Nil(t, err)
@@ -1229,7 +1229,7 @@ func testPendingAutoAddChannelMembers(t *testing.T, ss store.Store) {
 	require.Len(t, channelMembers, 0)
 
 	// restore group member and verify
-	res = <-ss.Group().CreateOrRestoreMember(group.Id, user.Id)
+	res = <-ss.Group().UpsertMember(group.Id, user.Id)
 	require.Nil(t, res.Err)
 	channelMembers, err = ss.Group().ChannelMembersToAdd(0)
 	require.Nil(t, err)
@@ -1458,10 +1458,10 @@ func pendingMemberRemovalsDataSetup(t *testing.T, ss store.Store) *removalsData
 	userC = res.Data.(*model.User)
 
 	// add users to group (but not userC)
-	res = <-ss.Group().CreateOrRestoreMember(group.Id, userA.Id)
+	res = <-ss.Group().UpsertMember(group.Id, userA.Id)
 	require.Nil(t, res.Err)
 
-	res = <-ss.Group().CreateOrRestoreMember(group.Id, userB.Id)
+	res = <-ss.Group().UpsertMember(group.Id, userB.Id)
 	require.Nil(t, res.Err)
 
 	// create channels
@@ -1653,7 +1653,7 @@ func testGetGroupsByChannel(t *testing.T, ss store.Store) {
 	res = <-ss.User().Save(u1)
 	require.Nil(t, res.Err)
 	user1 := res.Data.(*model.User)
-	<-ss.Group().CreateOrRestoreMember(group1.Id, user1.Id)
+	<-ss.Group().UpsertMember(group1.Id, user1.Id)
 
 	group1WithMemberCount := model.Group(*group1)
 	group1WithMemberCount.MemberCount = model.NewInt(1)
@@ -1854,7 +1854,7 @@ func testGetGroupsByTeam(t *testing.T, ss store.Store) {
 	res = <-ss.User().Save(u1)
 	require.Nil(t, res.Err)
 	user1 := res.Data.(*model.User)
-	<-ss.Group().CreateOrRestoreMember(group1.Id, user1.Id)
+	<-ss.Group().UpsertMember(group1.Id, user1.Id)
 
 	group1WithMemberCount := model.Group(*group1)
 	group1WithMemberCount.MemberCount = model.NewInt(1)
@@ -2096,7 +2096,7 @@ func testGetGroups(t *testing.T, ss store.Store) {
 	res = <-ss.User().Save(u1)
 	require.Nil(t, res.Err)
 	user1 := res.Data.(*model.User)
-	<-ss.Group().CreateOrRestoreMember(group1.Id, user1.Id)
+	<-ss.Group().UpsertMember(group1.Id, user1.Id)
 
 	group1WithMemberCount := model.Group(*group1)
 	group1WithMemberCount.MemberCount = model.NewInt(1)
@@ -2299,11 +2299,11 @@ func testTeamMembersMinusGroupMembers(t *testing.T, ss store.Store) {
 	// Add even users to even group, and the inverse
 	for i := 0; i < numberOfUsers; i++ {
 		groupIndex := int(math.Mod(float64(i), 2))
-		res := <-ss.Group().CreateOrRestoreMember(groups[groupIndex].Id, users[i].Id)
+		res := <-ss.Group().UpsertMember(groups[groupIndex].Id, users[i].Id)
 		require.Nil(t, res.Err)
 
 		// Add everyone to group 2
-		res = <-ss.Group().CreateOrRestoreMember(groups[numberOfGroups-1].Id, users[i].Id)
+		res = <-ss.Group().UpsertMember(groups[numberOfGroups-1].Id, users[i].Id)
 		require.Nil(t, res.Err)
 	}
 
@@ -2453,11 +2453,11 @@ func testChannelMembersMinusGroupMembers(t *testing.T, ss store.Store) {
 	// Add even users to even group, and the inverse
 	for i := 0; i < numberOfUsers; i++ {
 		groupIndex := int(math.Mod(float64(i), 2))
-		res := <-ss.Group().CreateOrRestoreMember(groups[groupIndex].Id, users[i].Id)
+		res := <-ss.Group().UpsertMember(groups[groupIndex].Id, users[i].Id)
 		require.Nil(t, res.Err)
 
 		// Add everyone to group 2
-		res = <-ss.Group().CreateOrRestoreMember(groups[numberOfGroups-1].Id, users[i].Id)
+		res = <-ss.Group().UpsertMember(groups[numberOfGroups-1].Id, users[i].Id)
 		require.Nil(t, res.Err)
 	}
 

store/storetest/mocks/GroupStore.go

@@ -221,22 +221,6 @@ func (_m *GroupStore) CreateGroupSyncable(groupSyncable *model.GroupSyncable) (*
 	return r0, r1
 }
 
-// CreateOrRestoreMember provides a mock function with given fields: groupID, userID
-func (_m *GroupStore) CreateOrRestoreMember(groupID string, userID string) store.StoreChannel {
-	ret := _m.Called(groupID, userID)
-
-	var r0 store.StoreChannel
-	if rf, ok := ret.Get(0).(func(string, string) store.StoreChannel); ok {
-		r0 = rf(groupID, userID)
-	} else {
-		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
-		}
-	}
-
-	return r0
-}
-
 // Delete provides a mock function with given fields: groupID
 func (_m *GroupStore) Delete(groupID string) store.StoreChannel {
 	ret := _m.Called(groupID)
@@ -655,3 +639,19 @@ func (_m *GroupStore) UpdateGroupSyncable(groupSyncable *model.GroupSyncable) (*
 
 	return r0, r1
 }
+
+// UpsertMember provides a mock function with given fields: groupID, userID
+func (_m *GroupStore) UpsertMember(groupID string, userID string) store.StoreChannel {
+	ret := _m.Called(groupID, userID)
+
+	var r0 store.StoreChannel
+	if rf, ok := ret.Get(0).(func(string, string) store.StoreChannel); ok {
+		r0 = rf(groupID, userID)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(store.StoreChannel)
+		}
+	}
+
+	return r0
+}

store/storetest/user_store.go

@@ -1134,7 +1134,7 @@ func testUserStoreGetProfilesNotInChannel(t *testing.T, ss store.Store) {
 
 	// add two members to the group
 	for _, u := range []*model.User{u1, u2} {
-		res := <-ss.Group().CreateOrRestoreMember(group.Id, u.Id)
+		res := <-ss.Group().UpsertMember(group.Id, u.Id)
 		require.Nil(t, res.Err)
 	}
 
@@ -3467,7 +3467,7 @@ func testUserStoreGetProfilesNotInTeam(t *testing.T, ss store.Store) {
 
 	// add two members to the group
 	for _, u := range []*model.User{u1, u2} {
-		res := <-ss.Group().CreateOrRestoreMember(group.Id, u.Id)
+		res := <-ss.Group().UpsertMember(group.Id, u.Id)
 		require.Nil(t, res.Err)
 	}
 
@@ -3773,9 +3773,9 @@ func testUserStoreGetTeamGroupUsers(t *testing.T, ss store.Store) {
 	groupB := testGroups[1]
 
 	// add members to groups
-	res = <-ss.Group().CreateOrRestoreMember(groupA.Id, userGroupA.Id)
+	res = <-ss.Group().UpsertMember(groupA.Id, userGroupA.Id)
 	require.Nil(t, res.Err)
-	res = <-ss.Group().CreateOrRestoreMember(groupB.Id, userGroupB.Id)
+	res = <-ss.Group().UpsertMember(groupB.Id, userGroupB.Id)
 	require.Nil(t, res.Err)
 
 	// association one group to team
@@ -3895,9 +3895,9 @@ func testUserStoreGetChannelGroupUsers(t *testing.T, ss store.Store) {
 	groupB := testGroups[1]
 
 	// add members to groups
-	res = <-ss.Group().CreateOrRestoreMember(groupA.Id, userGroupA.Id)
+	res = <-ss.Group().UpsertMember(groupA.Id, userGroupA.Id)
 	require.Nil(t, res.Err)
-	res = <-ss.Group().CreateOrRestoreMember(groupB.Id, userGroupB.Id)
+	res = <-ss.Group().UpsertMember(groupB.Id, userGroupB.Id)
 	require.Nil(t, res.Err)
 
 	// association one group to channel