feat: sort users by username (#7838)

mtojek · web-flow · commit 93378daeb376 · 2023-06-06T08:47:59.000+02:00
diff --git a/coderd/database/dbauthz/querier_test.go b/coderd/database/dbauthz/querier_test.go
@@ -726,14 +726,14 @@ func (s *MethodTestSuite) TestUser() {
 		check.Args(database.GetFilteredUserCountParams{}).Asserts().Returns(int64(1))
 	}))
 	s.Run("GetUsers", s.Subtest(func(db database.Store, check *expects) {
-		a := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now().Add(-time.Hour)})
-		b := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now()})
+		a := dbgen.User(s.T(), db, database.User{Username: "GetUsers-a-user"})
+		b := dbgen.User(s.T(), db, database.User{Username: "GetUsers-b-user"})
 		check.Args(database.GetUsersParams{}).
 			Asserts(a, rbac.ActionRead, b, rbac.ActionRead)
 	}))
 	s.Run("GetUsersWithCount", s.Subtest(func(db database.Store, check *expects) {
-		a := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now().Add(-time.Hour)})
-		b := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now()})
+		a := dbgen.User(s.T(), db, database.User{Username: "GetUsersWithCount-a-user"})
+		b := dbgen.User(s.T(), db, database.User{Username: "GetUsersWithCount-b-user"})
 		check.Args(database.GetUsersParams{}).Asserts(a, rbac.ActionRead, b, rbac.ActionRead)
 	}))
 	s.Run("InsertUser", s.Subtest(func(db database.Store, check *expects) {
diff --git a/coderd/database/dbfake/databasefake.go b/coderd/database/dbfake/databasefake.go
@@ -931,14 +931,9 @@ func (q *fakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams
 	users := make([]database.User, len(q.users))
 	copy(users, q.users)
 
-	// Database orders by created_at
+	// Database orders by username
 	slices.SortFunc(users, func(a, b database.User) bool {
-		if a.CreatedAt.Equal(b.CreatedAt) {
-			// Technically the postgres database also orders by uuid. So match
-			// that behavior
-			return a.ID.String() < b.ID.String()
-		}
-		return a.CreatedAt.Before(b.CreatedAt)
+		return a.Username < b.Username
 	})
 
 	// Filter out deleted since they should never be returned..
diff --git a/coderd/database/dbfake/databasefake_test.go b/coderd/database/dbfake/databasefake_test.go
@@ -5,6 +5,7 @@ import (
 	"database/sql"
 	"fmt"
 	"reflect"
+	"sort"
 	"testing"
 	"time"
 
@@ -106,26 +107,25 @@ func TestExactMethods(t *testing.T) {
 	}
 }
 
-// TestUserOrder ensures that the fake database returns users in the order they
-// were created.
+// TestUserOrder ensures that the fake database returns users sorted by username.
 func TestUserOrder(t *testing.T) {
 	t.Parallel()
 
 	db := dbfake.New()
 	now := database.Now()
-	count := 10
-	for i := 0; i < count; i++ {
-		dbgen.User(t, db, database.User{
-			Username:  fmt.Sprintf("user%d", i),
-			CreatedAt: now,
-		})
+
+	usernames := []string{"b-user", "d-user", "a-user", "c-user", "e-user"}
+	for _, username := range usernames {
+		dbgen.User(t, db, database.User{Username: username, CreatedAt: now})
 	}
 
 	users, err := db.GetUsers(context.Background(), database.GetUsersParams{})
 	require.NoError(t, err)
-	require.Lenf(t, users, count, "expected %d users", count)
+	require.Lenf(t, users, len(usernames), "expected %d users", len(usernames))
+
+	sort.Strings(usernames)
 	for i, user := range users {
-		require.Equal(t, fmt.Sprintf("user%d", i), user.Username)
+		require.Equal(t, usernames[i], user.Username)
 	}
 }
 
diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql
@@ -143,11 +143,11 @@ WHERE
 		-- duplicating or missing data.
 		WHEN @after_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN (
 			-- The pagination cursor is the last ID of the previous page.
-			-- The query is ordered by the created_at field, so select all
+			-- The query is ordered by the username field, so select all
 			-- rows after the cursor.
-			(created_at, id) > (
+			(username) > (
 				SELECT
-					created_at, id
+					username
 				FROM
 					users
 				WHERE
@@ -183,9 +183,8 @@ WHERE
 	END
 	-- End of filters
 ORDER BY
-	-- Deterministic and consistent ordering of all users, even if they share
-	-- a timestamp. This is to ensure consistent pagination.
-	(created_at, id) ASC OFFSET @offset_opt
+	-- Deterministic and consistent ordering of all users. This is to ensure consistent pagination.
+	username ASC OFFSET @offset_opt
 LIMIT
 	-- A null limit means "no limit", so 0 means return all
 	NULLIF(@limit_opt :: int, 0);
diff --git a/coderd/users_test.go b/coderd/users_test.go
@@ -1579,10 +1579,7 @@ func TestPaginatedUsers(t *testing.T) {
 	err = eg.Wait()
 	require.NoError(t, err, "create users failed")
 
-	// Sorting the users will sort by (created_at, uuid). This is to handle
-	// the off case that created_at is identical for 2 users.
-	// This is a really rare case in production, but does happen in unit tests
-	// due to the fake database being in memory and exceptionally quick.
+	// Sorting the users will sort by username.
 	sortUsers(allUsers)
 	sortUsers(specialUsers)
 
@@ -1697,10 +1694,7 @@ func assertPagination(ctx context.Context, t *testing.T, client *codersdk.Client
 // sortUsers sorts by (created_at, id)
 func sortUsers(users []codersdk.User) {
 	sort.Slice(users, func(i, j int) bool {
-		if users[i].CreatedAt.Equal(users[j].CreatedAt) {
-			return users[i].ID.String() < users[j].ID.String()
-		}
-		return users[i].CreatedAt.Before(users[j].CreatedAt)
+		return users[i].Username < users[j].Username
 	})
 }
 
