diff --git a/api4/channel_test.go b/api4/channel_test.go index f1ca573335..a367471460 100644 --- a/api4/channel_test.go +++ b/api4/channel_test.go @@ -2269,7 +2269,7 @@ func TestAddChannelMember(t *testing.T) { CheckErrorMessage(t, resp, "api.channel.add_members.user_denied") // Associate group to team - _, appErr = th.App.CreateGroupSyncable(&model.GroupSyncable{ + _, appErr = th.App.UpsertGroupSyncable(&model.GroupSyncable{ GroupId: th.Group.Id, SyncableId: privateChannel.Id, Type: model.GroupSyncableTypeChannel, diff --git a/api4/group.go b/api4/group.go index 42d3fad8f6..f39045e74b 100644 --- a/api4/group.go +++ b/api4/group.go @@ -4,7 +4,6 @@ package api4 import ( - "database/sql" "encoding/json" "fmt" "io/ioutil" @@ -181,34 +180,18 @@ func linkGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) { return } - groupSyncable, appErr := c.App.GetGroupSyncable(c.Params.GroupId, syncableID, syncableType) - if appErr != nil && appErr.DetailedError != sql.ErrNoRows.Error() { + groupSyncable := &model.GroupSyncable{ + GroupId: c.Params.GroupId, + SyncableId: syncableID, + Type: syncableType, + } + groupSyncable.Patch(patch) + groupSyncable, appErr = c.App.UpsertGroupSyncable(groupSyncable) + if appErr != nil { c.Err = appErr return } - if groupSyncable == nil { - groupSyncable = &model.GroupSyncable{ - GroupId: c.Params.GroupId, - SyncableId: syncableID, - Type: syncableType, - } - groupSyncable.Patch(patch) - groupSyncable, appErr = c.App.CreateGroupSyncable(groupSyncable) - if appErr != nil { - c.Err = appErr - return - } - } else { - groupSyncable.DeleteAt = 0 - groupSyncable.Patch(patch) - groupSyncable, appErr = c.App.UpdateGroupSyncable(groupSyncable) - if appErr != nil { - c.Err = appErr - return - } - } - w.WriteHeader(http.StatusCreated) b, marshalErr := json.Marshal(groupSyncable) diff --git a/api4/group_test.go b/api4/group_test.go index 425ce2cf3b..1006a2edc3 100644 --- a/api4/group_test.go +++ b/api4/group_test.go @@ -645,7 +645,7 @@ func TestGetGroupsByChannel(t *testing.T) { }) assert.Nil(t, err) - _, err = th.App.CreateGroupSyncable(&model.GroupSyncable{ + _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ AutoAdd: true, SyncableId: th.BasicChannel.Id, Type: model.GroupSyncableTypeChannel, @@ -698,7 +698,7 @@ func TestGetGroupsByTeam(t *testing.T) { }) assert.Nil(t, err) - _, err = th.App.CreateGroupSyncable(&model.GroupSyncable{ + _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ AutoAdd: true, SyncableId: th.BasicTeam.Id, Type: model.GroupSyncableTypeTeam, diff --git a/api4/team_test.go b/api4/team_test.go index 384327dced..4e793df6f0 100644 --- a/api4/team_test.go +++ b/api4/team_test.go @@ -1546,7 +1546,7 @@ func TestAddTeamMember(t *testing.T) { CheckErrorMessage(t, resp, "api.team.add_members.user_denied") // Associate group to team - _, err = th.App.CreateGroupSyncable(&model.GroupSyncable{ + _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ GroupId: th.Group.Id, SyncableId: team.Id, Type: model.GroupSyncableTypeTeam, @@ -1754,7 +1754,7 @@ func TestAddTeamMembers(t *testing.T) { CheckErrorMessage(t, resp, "api.team.add_members.user_denied") // Associate group to team - _, err = th.App.CreateGroupSyncable(&model.GroupSyncable{ + _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ GroupId: th.Group.Id, SyncableId: team.Id, Type: model.GroupSyncableTypeTeam, diff --git a/app/group.go b/app/group.go index 76ae9ed5e0..0f22d0d76b 100644 --- a/app/group.go +++ b/app/group.go @@ -64,8 +64,42 @@ func (a *App) DeleteGroupMember(groupID string, userID string) (*model.GroupMemb return a.Srv.Store.Group().DeleteMember(groupID, userID) } -func (a *App) CreateGroupSyncable(groupSyncable *model.GroupSyncable) (*model.GroupSyncable, *model.AppError) { - return a.Srv.Store.Group().CreateGroupSyncable(groupSyncable) +func (a *App) UpsertGroupSyncable(groupSyncable *model.GroupSyncable) (*model.GroupSyncable, *model.AppError) { + gs, err := a.Srv.Store.Group().GetGroupSyncable(groupSyncable.GroupId, groupSyncable.SyncableId, groupSyncable.Type) + if err != nil && err.Id != "store.sql_group.no_rows" { + return nil, err + } + + if gs == nil { + gs, err = a.Srv.Store.Group().CreateGroupSyncable(groupSyncable) + if err != nil { + return nil, err + } + } else { + gs, err = a.Srv.Store.Group().UpdateGroupSyncable(groupSyncable) + if err != nil { + return nil, err + } + } + + // if the type is channel, then upsert the associated GroupTeam [MM-14675] + if gs.Type == model.GroupSyncableTypeChannel { + channel, err := a.Srv.Store.Channel().Get(gs.SyncableId, true) + if err != nil { + return nil, err + } + _, err = a.UpsertGroupSyncable(&model.GroupSyncable{ + GroupId: gs.GroupId, + SyncableId: channel.TeamId, + Type: model.GroupSyncableTypeTeam, + AutoAdd: gs.AutoAdd, + }) + if err != nil { + return nil, err + } + } + + return gs, nil } func (a *App) GetGroupSyncable(groupID string, syncableID string, syncableType model.GroupSyncableType) (*model.GroupSyncable, *model.AppError) { @@ -77,11 +111,45 @@ func (a *App) GetGroupSyncables(groupID string, syncableType model.GroupSyncable } func (a *App) UpdateGroupSyncable(groupSyncable *model.GroupSyncable) (*model.GroupSyncable, *model.AppError) { - return a.Srv.Store.Group().UpdateGroupSyncable(groupSyncable) + var gs *model.GroupSyncable + var err *model.AppError + + if groupSyncable.DeleteAt == 0 { + // updating a *deleted* GroupSyncable, so no need to ensure the GroupTeam is present (as done in the upsert) + gs, err = a.Srv.Store.Group().UpdateGroupSyncable(groupSyncable) + } else { + // do an upsert to ensure that there's an associated GroupTeam + gs, err = a.UpsertGroupSyncable(groupSyncable) + } + if err != nil { + return nil, err + } + + return gs, nil } func (a *App) DeleteGroupSyncable(groupID string, syncableID string, syncableType model.GroupSyncableType) (*model.GroupSyncable, *model.AppError) { - return a.Srv.Store.Group().DeleteGroupSyncable(groupID, syncableID, syncableType) + gs, err := a.Srv.Store.Group().DeleteGroupSyncable(groupID, syncableID, syncableType) + if err != nil { + return nil, err + } + + // if a GroupTeam is being deleted delete all associated GroupChannels + if gs.Type == model.GroupSyncableTypeTeam { + allGroupChannels, err := a.Srv.Store.Group().GetAllGroupSyncablesByGroupId(gs.GroupId, model.GroupSyncableTypeChannel) + if err != nil { + return nil, err + } + + for _, groupChannel := range allGroupChannels { + _, err = a.Srv.Store.Group().DeleteGroupSyncable(groupChannel.GroupId, groupChannel.SyncableId, groupChannel.Type) + if err != nil { + return nil, err + } + } + } + + return gs, nil } func (a *App) TeamMembersToAdd(since int64) ([]*model.UserTeamIDPair, *model.AppError) { diff --git a/app/group_test.go b/app/group_test.go index e0da9e4843..0a8ca75b66 100644 --- a/app/group_test.go +++ b/app/group_test.go @@ -131,19 +131,30 @@ func TestDeleteGroupMember(t *testing.T) { require.Nil(t, groupMember) } -func TestCreateGroupSyncable(t *testing.T) { +func TestUpsertGroupSyncable(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() group := th.CreateGroup() groupSyncable := model.NewGroupTeam(group.Id, th.BasicTeam.Id, false) - gs, err := th.App.CreateGroupSyncable(groupSyncable) + gs, err := th.App.UpsertGroupSyncable(groupSyncable) require.Nil(t, err) require.NotNil(t, gs) - gs, err = th.App.CreateGroupSyncable(groupSyncable) - require.NotNil(t, err) - require.Nil(t, gs) + // can update again without error + gs, err = th.App.UpsertGroupSyncable(groupSyncable) + require.Nil(t, err) + require.NotNil(t, gs) + + gs, err = th.App.DeleteGroupSyncable(gs.GroupId, gs.SyncableId, gs.Type) + require.Nil(t, err) + require.NotEqual(t, int64(0), gs.DeleteAt) + + // Un-deleting works + gs.DeleteAt = 0 + gs, err = th.App.UpsertGroupSyncable(gs) + require.Nil(t, err) + require.Equal(t, int64(0), gs.DeleteAt) } func TestGetGroupSyncable(t *testing.T) { @@ -152,7 +163,7 @@ func TestGetGroupSyncable(t *testing.T) { group := th.CreateGroup() groupSyncable := model.NewGroupTeam(group.Id, th.BasicTeam.Id, false) - gs, err := th.App.CreateGroupSyncable(groupSyncable) + gs, err := th.App.UpsertGroupSyncable(groupSyncable) require.Nil(t, err) require.NotNil(t, gs) @@ -169,7 +180,7 @@ func TestGetGroupSyncables(t *testing.T) { // Create a group team groupSyncable := model.NewGroupTeam(group.Id, th.BasicTeam.Id, false) - gs, err := th.App.CreateGroupSyncable(groupSyncable) + gs, err := th.App.UpsertGroupSyncable(groupSyncable) require.Nil(t, err) require.NotNil(t, gs) @@ -185,7 +196,7 @@ func TestDeleteGroupSyncable(t *testing.T) { group := th.CreateGroup() groupChannel := model.NewGroupChannel(group.Id, th.BasicChannel.Id, false) - gs, err := th.App.CreateGroupSyncable(groupChannel) + gs, err := th.App.UpsertGroupSyncable(groupChannel) require.Nil(t, err) require.NotNil(t, gs) @@ -211,7 +222,7 @@ func TestGetGroupsByChannel(t *testing.T) { Type: model.GroupSyncableTypeChannel, } - gs, err := th.App.CreateGroupSyncable(groupSyncable) + gs, err := th.App.UpsertGroupSyncable(groupSyncable) require.Nil(t, err) require.NotNil(t, gs) @@ -244,7 +255,7 @@ func TestGetGroupsByTeam(t *testing.T) { Type: model.GroupSyncableTypeTeam, } - gs, err := th.App.CreateGroupSyncable(groupSyncable) + gs, err := th.App.UpsertGroupSyncable(groupSyncable) require.Nil(t, err) require.NotNil(t, gs) diff --git a/app/notification_test.go b/app/notification_test.go index 686f3a1f26..fbd6899bde 100644 --- a/app/notification_test.go +++ b/app/notification_test.go @@ -275,7 +275,7 @@ func TestFilterOutOfChannelMentions(t *testing.T) { constrainedChannel, appErr = th.App.UpdateChannel(constrainedChannel) require.Nil(t, appErr) - _, appErr = th.App.CreateGroupSyncable(&model.GroupSyncable{ + _, appErr = th.App.UpsertGroupSyncable(&model.GroupSyncable{ GroupId: group.Id, Type: model.GroupSyncableTypeChannel, SyncableId: constrainedChannel.Id, diff --git a/app/syncables_test.go b/app/syncables_test.go index 995ef1d2bf..e6e2272400 100644 --- a/app/syncables_test.go +++ b/app/syncables_test.go @@ -74,17 +74,17 @@ func TestCreateDefaultMemberships(t *testing.T) { t.Errorf("test group not created: %s", err.Error()) } - _, err = th.App.CreateGroupSyncable(model.NewGroupChannel(gleeGroup.Id, practiceChannel.Id, true)) + _, err = th.App.UpsertGroupSyncable(model.NewGroupChannel(gleeGroup.Id, practiceChannel.Id, true)) if err != nil { t.Errorf("test groupchannel not created: %s", err.Error()) } - scienceTeamGroupSyncable, err := th.App.CreateGroupSyncable(model.NewGroupTeam(scienceGroup.Id, nerdsTeam.Id, false)) + scienceTeamGroupSyncable, err := th.App.UpsertGroupSyncable(model.NewGroupTeam(scienceGroup.Id, nerdsTeam.Id, false)) if err != nil { t.Errorf("test groupteam not created: %s", err.Error()) } - scienceChannelGroupSyncable, err := th.App.CreateGroupSyncable(model.NewGroupChannel(scienceGroup.Id, experimentsChannel.Id, false)) + scienceChannelGroupSyncable, err := th.App.UpsertGroupSyncable(model.NewGroupChannel(scienceGroup.Id, experimentsChannel.Id, false)) if err != nil { t.Errorf("test groupchannel not created: %s", err.Error()) } @@ -363,9 +363,9 @@ func TestDeleteGroupMemberships(t *testing.T) { require.True(t, *channel.GroupConstrained) // create groupteam and groupchannel - _, err = th.App.CreateGroupSyncable(model.NewGroupTeam(group.Id, team.Id, true)) + _, err = th.App.UpsertGroupSyncable(model.NewGroupTeam(group.Id, team.Id, true)) require.Nil(t, err) - _, err = th.App.CreateGroupSyncable(model.NewGroupChannel(group.Id, channel.Id, true)) + _, err = th.App.UpsertGroupSyncable(model.NewGroupChannel(group.Id, channel.Id, true)) require.Nil(t, err) // verify the member count diff --git a/cmd/mattermost/commands/group_test.go b/cmd/mattermost/commands/group_test.go index dcdb83197a..bcf87a9d92 100644 --- a/cmd/mattermost/commands/group_test.go +++ b/cmd/mattermost/commands/group_test.go @@ -36,7 +36,7 @@ func TestChannelGroupEnable(t *testing.T) { }) require.Nil(t, err) - _, err = th.App.CreateGroupSyncable(&model.GroupSyncable{ + _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ AutoAdd: true, SyncableId: channel.Id, Type: model.GroupSyncableTypeChannel, @@ -80,7 +80,7 @@ func TestChannelGroupDisable(t *testing.T) { }) require.Nil(t, err) - _, err = th.App.CreateGroupSyncable(&model.GroupSyncable{ + _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ AutoAdd: true, SyncableId: channel.Id, Type: model.GroupSyncableTypeChannel, @@ -127,7 +127,7 @@ func TestChannelGroupStatus(t *testing.T) { }) require.Nil(t, err) - _, err = th.App.CreateGroupSyncable(&model.GroupSyncable{ + _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ AutoAdd: true, SyncableId: channel.Id, Type: model.GroupSyncableTypeChannel, @@ -170,7 +170,7 @@ func TestChannelGroupList(t *testing.T) { }) require.Nil(t, err) - _, err = th.App.CreateGroupSyncable(&model.GroupSyncable{ + _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ AutoAdd: true, SyncableId: channel.Id, Type: model.GroupSyncableTypeChannel, @@ -188,7 +188,7 @@ func TestChannelGroupList(t *testing.T) { }) require.Nil(t, err) - _, err = th.App.CreateGroupSyncable(&model.GroupSyncable{ + _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ AutoAdd: true, SyncableId: channel.Id, Type: model.GroupSyncableTypeChannel, @@ -229,7 +229,7 @@ func TestTeamGroupEnable(t *testing.T) { }) require.Nil(t, err) - _, err = th.App.CreateGroupSyncable(&model.GroupSyncable{ + _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ AutoAdd: true, SyncableId: th.BasicTeam.Id, Type: model.GroupSyncableTypeTeam, @@ -270,7 +270,7 @@ func TestTeamGroupDisable(t *testing.T) { }) require.Nil(t, err) - _, err = th.App.CreateGroupSyncable(&model.GroupSyncable{ + _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ AutoAdd: true, SyncableId: team.Id, Type: model.GroupSyncableTypeTeam, @@ -314,7 +314,7 @@ func TestTeamGroupStatus(t *testing.T) { }) require.Nil(t, err) - _, err = th.App.CreateGroupSyncable(&model.GroupSyncable{ + _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ AutoAdd: true, SyncableId: th.BasicTeam.Id, Type: model.GroupSyncableTypeTeam, @@ -354,7 +354,7 @@ func TestTeamGroupList(t *testing.T) { }) require.Nil(t, err) - _, err = th.App.CreateGroupSyncable(&model.GroupSyncable{ + _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ AutoAdd: true, SyncableId: th.BasicTeam.Id, Type: model.GroupSyncableTypeTeam, @@ -372,7 +372,7 @@ func TestTeamGroupList(t *testing.T) { }) require.Nil(t, err) - _, err = th.App.CreateGroupSyncable(&model.GroupSyncable{ + _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ AutoAdd: true, SyncableId: th.BasicTeam.Id, Type: model.GroupSyncableTypeTeam, diff --git a/store/sqlstore/group_store.go b/store/sqlstore/group_store.go index d5eb1e3d12..61b3ca6286 100644 --- a/store/sqlstore/group_store.go +++ b/store/sqlstore/group_store.go @@ -192,7 +192,7 @@ func (s *SqlGroupStore) GetByUser(userId string) ([]*model.Group, *model.AppErro func (s *SqlGroupStore) Update(group *model.Group) (*model.Group, *model.AppError) { var retrievedGroup *model.Group - if err := s.GetMaster().SelectOne(&retrievedGroup, "SELECT * FROM UserGroups WHERE Id = :Id", map[string]interface{}{"Id": group.Id}); err != nil { + if err := s.GetReplica().SelectOne(&retrievedGroup, "SELECT * FROM UserGroups WHERE Id = :Id", map[string]interface{}{"Id": group.Id}); err != nil { if err == sql.ErrNoRows { return nil, model.NewAppError("SqlGroupStore.GroupUpdate", "store.sql_group.no_rows", nil, "id="+group.Id+","+err.Error(), http.StatusNotFound) } @@ -322,12 +322,12 @@ func (s *SqlGroupStore) UpsertMember(groupID string, userID string) (*model.Grou } var retrievedGroup *model.Group - if err := s.GetMaster().SelectOne(&retrievedGroup, "SELECT * FROM UserGroups WHERE Id = :Id", map[string]interface{}{"Id": groupID}); err != nil { + if err := s.GetReplica().SelectOne(&retrievedGroup, "SELECT * FROM UserGroups WHERE Id = :Id", map[string]interface{}{"Id": groupID}); err != nil { return nil, model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.insert_error", nil, "group_id="+member.GroupId+"user_id="+member.UserId+","+err.Error(), http.StatusInternalServerError) } var retrievedMember *model.GroupMember - if err := s.GetMaster().SelectOne(&retrievedMember, "SELECT * FROM GroupMembers WHERE GroupId = :GroupId AND UserId = :UserId", map[string]interface{}{"GroupId": member.GroupId, "UserId": member.UserId}); err != nil { + if err := s.GetReplica().SelectOne(&retrievedMember, "SELECT * FROM GroupMembers WHERE GroupId = :GroupId AND UserId = :UserId", map[string]interface{}{"GroupId": member.GroupId, "UserId": member.UserId}); err != nil { if err != sql.ErrNoRows { return nil, model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.select_error", nil, "group_id="+member.GroupId+"user_id="+member.UserId+","+err.Error(), http.StatusInternalServerError) } @@ -357,7 +357,7 @@ func (s *SqlGroupStore) UpsertMember(groupID string, userID string) (*model.Grou func (s *SqlGroupStore) DeleteMember(groupID string, userID string) (*model.GroupMember, *model.AppError) { var retrievedMember *model.GroupMember - if err := s.GetMaster().SelectOne(&retrievedMember, "SELECT * FROM GroupMembers WHERE GroupId = :GroupId AND UserId = :UserId AND DeleteAt = 0", map[string]interface{}{"GroupId": groupID, "UserId": userID}); err != nil { + if err := s.GetReplica().SelectOne(&retrievedMember, "SELECT * FROM GroupMembers WHERE GroupId = :GroupId AND UserId = :UserId AND DeleteAt = 0", map[string]interface{}{"GroupId": groupID, "UserId": userID}); err != nil { if err == sql.ErrNoRows { return nil, model.NewAppError("SqlGroupStore.GroupDeleteMember", "store.sql_group.no_rows", nil, "group_id="+groupID+"user_id="+userID+","+err.Error(), http.StatusNotFound) } @@ -434,9 +434,9 @@ func (s *SqlGroupStore) getGroupSyncable(groupID string, syncableID string, sync switch syncableType { case model.GroupSyncableTypeTeam: - result, err = s.GetMaster().Get(groupTeam{}, groupID, syncableID) + result, err = s.GetReplica().Get(groupTeam{}, groupID, syncableID) case model.GroupSyncableTypeChannel: - result, err = s.GetMaster().Get(groupChannel{}, groupID, syncableID) + result, err = s.GetReplica().Get(groupChannel{}, groupID, syncableID) } if err != nil { @@ -497,7 +497,7 @@ func (s *SqlGroupStore) GetAllGroupSyncablesByGroupId(groupID string, syncableTy GroupId = :GroupId AND GroupTeams.DeleteAt = 0` results := []*groupTeamJoin{} - _, err := s.GetMaster().Select(&results, sqlQuery, args) + _, err := s.GetReplica().Select(&results, sqlQuery, args) if err != nil { return nil, appErrF(err.Error()) } @@ -532,7 +532,7 @@ func (s *SqlGroupStore) GetAllGroupSyncablesByGroupId(groupID string, syncableTy GroupId = :GroupId AND GroupChannels.DeleteAt = 0` results := []*groupChannelJoin{} - _, err := s.GetMaster().Select(&results, sqlQuery, args) + _, err := s.GetReplica().Select(&results, sqlQuery, args) if err != nil { return nil, appErrF(err.Error()) }