Add parameter to return group members in order of display name (#21775)

2022-12-15 14:20:36 -05:00
--- a/store/opentracinglayer/opentracinglayer.go
+++ b/store/opentracinglayer/opentracinglayer.go
@@ -4501,6 +4501,24 @@ func (s *OpenTracingLayerGroupStore) GetMemberUsersPage(groupID string, page int
 	return result, err
 }

+func (s *OpenTracingLayerGroupStore) GetMemberUsersSortedPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions, teammateNameDisplay string) ([]*model.User, error) {
+	origCtx := s.Root.Store.Context()
+	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "GroupStore.GetMemberUsersSortedPage")
+	s.Root.Store.SetContext(newCtx)
+	defer func() {
+		s.Root.Store.SetContext(origCtx)
+	}()
+
+	defer span.Finish()
+	result, err := s.GroupStore.GetMemberUsersSortedPage(groupID, page, perPage, viewRestrictions, teammateNameDisplay)
+	if err != nil {
+		span.LogFields(spanlog.Error(err))
+		ext.Error.Set(span, true)
+	}
+
+	return result, err
+}
+
 func (s *OpenTracingLayerGroupStore) GetNonMemberUsersPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "GroupStore.GetNonMemberUsersPage")
--- a/store/retrylayer/retrylayer.go
+++ b/store/retrylayer/retrylayer.go
@@ -5070,6 +5070,27 @@ func (s *RetryLayerGroupStore) GetMemberUsersPage(groupID string, page int, perP

 }

+func (s *RetryLayerGroupStore) GetMemberUsersSortedPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions, teammateNameDisplay string) ([]*model.User, error) {
+
+	tries := 0
+	for {
+		result, err := s.GroupStore.GetMemberUsersSortedPage(groupID, page, perPage, viewRestrictions, teammateNameDisplay)
+		if err == nil {
+			return result, nil
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+		timepkg.Sleep(100 * timepkg.Millisecond)
+	}
+
+}
+
 func (s *RetryLayerGroupStore) GetNonMemberUsersPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) {

 	tries := 0
--- a/store/sqlstore/group_store.go
+++ b/store/sqlstore/group_store.go
@@ -421,22 +421,56 @@ func (s *SqlGroupStore) GetMemberUsers(groupID string) ([]*model.User, error) {
 }

 func (s *SqlGroupStore) GetMemberUsersPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) {
+	return s.GetMemberUsersSortedPage(groupID, page, perPage, viewRestrictions, model.ShowUsername)
+}
+
+func (s *SqlGroupStore) GetMemberUsersSortedPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions, teammateNameDisplay string) ([]*model.User, error) {
 	groupMembers := []*model.User{}

-	query := s.getQueryBuilder().
-		Select("u.*").
+	userQuery := s.getQueryBuilder().
+		Select(`u.*`).
 		From("GroupMembers").
 		Join("Users u ON u.Id = GroupMembers.UserId").
 		Where(sq.Eq{"GroupMembers.DeleteAt": 0}).
 		Where(sq.Eq{"u.DeleteAt": 0}).
-		Where(sq.Eq{"GroupId": groupID}).
+		Where(sq.Eq{"GroupId": groupID})
+
+	userQuery = applyViewRestrictionsFilter(userQuery, viewRestrictions, true)
+	queryString, args, err := userQuery.ToSql()
+	if err != nil {
+		return nil, errors.Wrap(err, "")
+	}
+
+	orderQuery := s.getQueryBuilder().
+		Select("u.*").
+		From("(" + queryString + ") AS u")
+
+	if teammateNameDisplay == model.ShowNicknameFullName {
+		orderQuery = orderQuery.OrderBy(`
+		CASE
+			WHEN u.Nickname != '' THEN u.Nickname
+			WHEN u.FirstName !=  '' AND u.LastName != '' THEN CONCAT(u.FirstName, ' ', u.LastName)
+			WHEN u.FirstName != '' THEN u.FirstName
+			WHEN u.LastName != '' THEN u.LastName
+			ELSE u.Username
+		END`)
+	} else if teammateNameDisplay == model.ShowFullName {
+		orderQuery = orderQuery.OrderBy(`
+		CASE
+			WHEN u.FirstName !=  '' AND u.LastName != '' THEN CONCAT(u.FirstName, ' ', u.LastName)
+			WHEN u.FirstName != '' THEN u.FirstName
+			WHEN u.LastName != '' THEN u.LastName
+			ELSE u.Username
+		END`)
+	} else {
+		orderQuery = orderQuery.OrderBy("u.Username")
+	}
+
+	orderQuery = orderQuery.
 		Limit(uint64(perPage)).
-		Offset(uint64(page * perPage)).
-		OrderBy("u.CreateAt DESC")
+		Offset(uint64(page * perPage))

-	query = applyViewRestrictionsFilter(query, viewRestrictions, true)
-
-	queryString, args, err := query.ToSql()
+	queryString, _, err = orderQuery.ToSql()
 	if err != nil {
 		return nil, errors.Wrap(err, "")
 	}
@@ -463,7 +497,7 @@ func (s *SqlGroupStore) GetNonMemberUsersPage(groupID string, page int, perPage
 		Where("(GroupMembers.UserID IS NULL OR GroupMembers.DeleteAt != 0)").
 		Limit(uint64(perPage)).
 		Offset(uint64(page * perPage)).
-		OrderBy("u.CreateAt DESC")
+		OrderBy("u.Username ASC")

 	query = applyViewRestrictionsFilter(query, viewRestrictions, true)

--- a/store/store.go
+++ b/store/store.go
@@ -842,6 +842,7 @@ type GroupStore interface {

 	GetMemberUsers(groupID string) ([]*model.User, error)
 	GetMemberUsersPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error)
+	GetMemberUsersSortedPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions, teammateNameDisplay string) ([]*model.User, error)
 	GetMemberCountWithRestrictions(groupID string, viewRestrictions *model.ViewUsersRestrictions) (int64, error)
 	GetMemberCount(groupID string) (int64, error)

--- a/store/storetest/group_store.go
+++ b/store/storetest/group_store.go
@@ -36,6 +36,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("GetMemberUsersSortedPage", func(t *testing.T) { testGroupGetMemberUsersSortedPage(t, ss) })

 	t.Run("GetMemberUsersInTeam", func(t *testing.T) { testGroupGetMemberUsersInTeam(t, ss) })
 	t.Run("GetMemberUsersNotInChannel", func(t *testing.T) { testGroupGetMemberUsersNotInChannel(t, ss) })
@@ -861,7 +862,7 @@ func testGroupGetMemberUsersPage(t *testing.T, ss store.Store) {

 	u1 := &model.User{
 		Email:    MakeEmail(),
-		Username: model.NewId(),
+		Username: "user1" + model.NewId(),
 	}
 	user1, nErr := ss.User().Save(u1)
 	require.NoError(t, nErr)
@@ -871,7 +872,7 @@ func testGroupGetMemberUsersPage(t *testing.T, ss store.Store) {

 	u2 := &model.User{
 		Email:    MakeEmail(),
-		Username: model.NewId(),
+		Username: "user2" + model.NewId(),
 	}
 	user2, nErr := ss.User().Save(u2)
 	require.NoError(t, nErr)
@@ -881,7 +882,7 @@ func testGroupGetMemberUsersPage(t *testing.T, ss store.Store) {

 	u3 := &model.User{
 		Email:    MakeEmail(),
-		Username: model.NewId(),
+		Username: "user3" + model.NewId(),
 	}
 	user3, nErr := ss.User().Save(u3)
 	require.NoError(t, nErr)
@@ -898,13 +899,13 @@ func testGroupGetMemberUsersPage(t *testing.T, ss store.Store) {
 	groupMembers, err = ss.Group().GetMemberUsersPage(group.Id, 0, 2, nil)
 	require.NoError(t, err)
 	require.Equal(t, 2, len(groupMembers))
-	require.ElementsMatch(t, []*model.User{user2, user3}, groupMembers)
+	require.ElementsMatch(t, []*model.User{user1, user2}, groupMembers)

 	// Check page 2
 	groupMembers, err = ss.Group().GetMemberUsersPage(group.Id, 1, 2, nil)
 	require.NoError(t, err)
 	require.Equal(t, 1, len(groupMembers))
-	require.ElementsMatch(t, []*model.User{user1}, groupMembers)
+	require.ElementsMatch(t, []*model.User{user3}, groupMembers)

 	// Check madeup id
 	groupMembers, err = ss.Group().GetMemberUsersPage(model.NewId(), 0, 100, nil)
@@ -921,6 +922,87 @@ func testGroupGetMemberUsersPage(t *testing.T, ss store.Store) {
 	require.Equal(t, 2, len(groupMembers))
 }

+func testGroupGetMemberUsersSortedPage(t *testing.T, ss store.Store) {
+	// Save a group
+	g1 := &model.Group{
+		Name:        model.NewString(model.NewId()),
+		DisplayName: model.NewId(),
+		Description: model.NewId(),
+		Source:      model.GroupSourceLdap,
+		RemoteId:    model.NewString(model.NewId()),
+	}
+	group, err := ss.Group().Create(g1)
+	require.NoError(t, err)
+
+	// First by nickname, third by full name, second by username
+	u1 := &model.User{
+		Email:     MakeEmail(),
+		Username:  "y" + model.NewId(),
+		Nickname:  "a" + model.NewId(),
+		FirstName: "z" + model.NewId(),
+		LastName:  "z" + model.NewId(),
+	}
+	user1, nErr := ss.User().Save(u1)
+	require.NoError(t, nErr)
+
+	_, err = ss.Group().UpsertMember(group.Id, user1.Id)
+	require.NoError(t, err)
+
+	// Second by nickname, first by full name, third by username
+	u2 := &model.User{
+		Email:     MakeEmail(),
+		Username:  "z" + model.NewId(),
+		FirstName: "b" + model.NewId(),
+		LastName:  "b" + model.NewId(),
+	}
+	user2, nErr := ss.User().Save(u2)
+	require.NoError(t, nErr)
+
+	_, err = ss.Group().UpsertMember(group.Id, user2.Id)
+	require.NoError(t, err)
+
+	// Third by nickname, second by full name, first by username
+	u3 := &model.User{
+		Email:    MakeEmail(),
+		Username: "d" + model.NewId(),
+	}
+	user3, nErr := ss.User().Save(u3)
+	require.NoError(t, nErr)
+
+	_, err = ss.Group().UpsertMember(group.Id, user3.Id)
+	require.NoError(t, err)
+
+	// Check nickname ordering, paged
+	groupMembers, err := ss.Group().GetMemberUsersSortedPage(group.Id, 0, 2, nil, model.ShowNicknameFullName)
+	require.NoError(t, err)
+	require.Equal(t, 2, len(groupMembers))
+	require.ElementsMatch(t, []*model.User{user1, user2}, groupMembers)
+	groupMembers, err = ss.Group().GetMemberUsersSortedPage(group.Id, 1, 2, nil, model.ShowNicknameFullName)
+	require.NoError(t, err)
+	require.Equal(t, 1, len(groupMembers))
+	require.ElementsMatch(t, []*model.User{user3}, groupMembers)
+
+	// Check full name ordering, paged
+	groupMembers, err = ss.Group().GetMemberUsersSortedPage(group.Id, 0, 2, nil, model.ShowFullName)
+	require.NoError(t, err)
+	require.Equal(t, 2, len(groupMembers))
+	require.ElementsMatch(t, []*model.User{user2, user3}, groupMembers)
+	groupMembers, err = ss.Group().GetMemberUsersSortedPage(group.Id, 1, 2, nil, model.ShowFullName)
+	require.NoError(t, err)
+	require.Equal(t, 1, len(groupMembers))
+	require.ElementsMatch(t, []*model.User{user1}, groupMembers)
+
+	// Check username ordering
+	groupMembers, err = ss.Group().GetMemberUsersSortedPage(group.Id, 0, 2, nil, model.ShowUsername)
+	require.NoError(t, err)
+	require.Equal(t, 2, len(groupMembers))
+	require.ElementsMatch(t, []*model.User{user3, user1}, groupMembers)
+	groupMembers, err = ss.Group().GetMemberUsersSortedPage(group.Id, 1, 2, nil, model.ShowUsername)
+	require.NoError(t, err)
+	require.Equal(t, 1, len(groupMembers))
+	require.ElementsMatch(t, []*model.User{user2}, groupMembers)
+}
+
 func testGroupGetMemberUsersInTeam(t *testing.T, ss store.Store) {
 	// Save a team
 	team := &model.Team{
--- a/store/storetest/mocks/GroupStore.go
+++ b/store/storetest/mocks/GroupStore.go
@@ -826,6 +826,29 @@ func (_m *GroupStore) GetMemberUsersPage(groupID string, page int, perPage int,
 	return r0, r1
 }

+// GetMemberUsersSortedPage provides a mock function with given fields: groupID, page, perPage, viewRestrictions, teammateNameDisplay
+func (_m *GroupStore) GetMemberUsersSortedPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions, teammateNameDisplay string) ([]*model.User, error) {
+	ret := _m.Called(groupID, page, perPage, viewRestrictions, teammateNameDisplay)
+
+	var r0 []*model.User
+	if rf, ok := ret.Get(0).(func(string, int, int, *model.ViewUsersRestrictions, string) []*model.User); ok {
+		r0 = rf(groupID, page, perPage, viewRestrictions, teammateNameDisplay)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).([]*model.User)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(string, int, int, *model.ViewUsersRestrictions, string) error); ok {
+		r1 = rf(groupID, page, perPage, viewRestrictions, teammateNameDisplay)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
 // GetNonMemberUsersPage provides a mock function with given fields: groupID, page, perPage, viewRestrictions
 func (_m *GroupStore) GetNonMemberUsersPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) {
 	ret := _m.Called(groupID, page, perPage, viewRestrictions)
--- a/store/timerlayer/timerlayer.go
+++ b/store/timerlayer/timerlayer.go
@@ -4098,6 +4098,22 @@ func (s *TimerLayerGroupStore) GetMemberUsersPage(groupID string, page int, perP
 	return result, err
 }

+func (s *TimerLayerGroupStore) GetMemberUsersSortedPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions, teammateNameDisplay string) ([]*model.User, error) {
+	start := time.Now()
+
+	result, err := s.GroupStore.GetMemberUsersSortedPage(groupID, page, perPage, viewRestrictions, teammateNameDisplay)
+
+	elapsed := float64(time.Since(start)) / float64(time.Second)
+	if s.Root.Metrics != nil {
+		success := "false"
+		if err == nil {
+			success = "true"
+		}
+		s.Root.Metrics.ObserveStoreMethodDuration("GroupStore.GetMemberUsersSortedPage", success, elapsed)
+	}
+	return result, err
+}
+
 func (s *TimerLayerGroupStore) GetNonMemberUsersPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) {
 	start := time.Now()