Add some basic sorting support for GET /users endpoint (#6801)

2017-06-30 12:07:23 -04:00
--- a/api4/user.go
+++ b/api4/user.go
@@ -277,9 +277,21 @@ func getUsers(c *Context, w http.ResponseWriter, r *http.Request) {
 	inChannelId := r.URL.Query().Get("in_channel")
 	notInChannelId := r.URL.Query().Get("not_in_channel")
 	withoutTeam := r.URL.Query().Get("without_team")
+	sort := r.URL.Query().Get("sort")

 	if len(notInChannelId) > 0 && len(inTeamId) == 0 {
-		c.SetInvalidParam("team_id")
+		c.SetInvalidUrlParam("team_id")
+		return
+	}
+
+	if sort != "" && sort != "last_activity_at" && sort != "create_at" {
+		c.SetInvalidUrlParam("sort")
+		return
+	}
+
+	// Currently only supports sorting on a team
+	if (sort == "last_activity_at" || sort == "create_at") && (inTeamId == "" || notInTeamId != "" || inChannelId != "" || notInChannelId != "" || withoutTeam != "") {
+		c.SetInvalidUrlParam("sort")
 		return
 	}

@@ -287,7 +299,7 @@ func getUsers(c *Context, w http.ResponseWriter, r *http.Request) {
 	var err *model.AppError
 	etag := ""

-	if withoutTeamBool, err := strconv.ParseBool(withoutTeam); err == nil && withoutTeamBool {
+	if withoutTeamBool, _ := strconv.ParseBool(withoutTeam); withoutTeamBool {
 		// Use a special permission for now
 		if !app.SessionHasPermissionTo(c.Session, model.PERMISSION_LIST_USERS_WITHOUT_TEAM) {
 			c.SetPermissionError(model.PERMISSION_LIST_USERS_WITHOUT_TEAM)
@@ -320,12 +332,18 @@ func getUsers(c *Context, w http.ResponseWriter, r *http.Request) {
 			return
 		}

-		etag = app.GetUsersInTeamEtag(inTeamId)
-		if HandleEtag(etag, "Get Users in Team", w, r) {
-			return
-		}
+		if sort == "last_activity_at" {
+			profiles, err = app.GetRecentlyActiveUsersForTeamPage(inTeamId, c.Params.Page, c.Params.PerPage, c.IsSystemAdmin())
+		} else if sort == "create_at" {
+			profiles, err = app.GetNewUsersForTeamPage(inTeamId, c.Params.Page, c.Params.PerPage, c.IsSystemAdmin())
+		} else {
+			etag = app.GetUsersInTeamEtag(inTeamId)
+			if HandleEtag(etag, "Get Users in Team", w, r) {
+				return
+			}

-		profiles, err = app.GetUsersInTeamPage(inTeamId, c.Params.Page, c.Params.PerPage, c.IsSystemAdmin())
+			profiles, err = app.GetUsersInTeamPage(inTeamId, c.Params.Page, c.Params.PerPage, c.IsSystemAdmin())
+		}
 	} else if len(inChannelId) > 0 {
 		if !app.SessionHasPermissionToChannel(c.Session, inChannelId, model.PERMISSION_READ_CHANNEL) {
 			c.SetPermissionError(model.PERMISSION_READ_CHANNEL)
--- a/api4/user_test.go
+++ b/api4/user_test.go
@@ -1224,6 +1224,64 @@ func TestGetUsers(t *testing.T) {
 	CheckUnauthorizedStatus(t, resp)
 }

+func TestGetNewUsersInTeam(t *testing.T) {
+	th := Setup().InitBasic()
+	defer TearDown()
+	Client := th.Client
+	teamId := th.BasicTeam.Id
+
+	rusers, resp := Client.GetNewUsersInTeam(teamId, 0, 60, "")
+	CheckNoError(t, resp)
+
+	lastCreateAt := model.GetMillis()
+	for _, u := range rusers {
+		if u.CreateAt > lastCreateAt {
+			t.Fatal("bad sorting")
+		}
+		lastCreateAt = u.CreateAt
+		CheckUserSanitization(t, u)
+	}
+
+	rusers, resp = Client.GetNewUsersInTeam(teamId, 1, 1, "")
+	CheckNoError(t, resp)
+	if len(rusers) != 1 {
+		t.Fatal("should be 1 per page")
+	}
+
+	Client.Logout()
+	_, resp = Client.GetNewUsersInTeam(teamId, 1, 1, "")
+	CheckUnauthorizedStatus(t, resp)
+}
+
+func TestGetRecentlyActiveUsersInTeam(t *testing.T) {
+	th := Setup().InitBasic()
+	defer TearDown()
+	Client := th.Client
+	teamId := th.BasicTeam.Id
+
+	app.SetStatusOnline(th.BasicUser.Id, "", true)
+
+	rusers, resp := Client.GetRecentlyActiveUsersInTeam(teamId, 0, 60, "")
+	CheckNoError(t, resp)
+
+	for _, u := range rusers {
+		if u.LastActivityAt == 0 {
+			t.Fatal("did not return last activity at")
+		}
+		CheckUserSanitization(t, u)
+	}
+
+	rusers, resp = Client.GetRecentlyActiveUsersInTeam(teamId, 0, 1, "")
+	CheckNoError(t, resp)
+	if len(rusers) != 1 {
+		t.Fatal("should be 1 per page")
+	}
+
+	Client.Logout()
+	_, resp = Client.GetRecentlyActiveUsersInTeam(teamId, 0, 1, "")
+	CheckUnauthorizedStatus(t, resp)
+}
+
 func TestGetUsersWithoutTeam(t *testing.T) {
 	th := Setup().InitBasic().InitSystemAdmin()
 	defer TearDown()
--- a/app/analytics.go
+++ b/app/analytics.go
@@ -231,9 +231,38 @@ func GetAnalytics(name string, teamId string) (model.AnalyticsRows, *model.AppEr
 }

 func GetRecentlyActiveUsersForTeam(teamId string) (map[string]*model.User, *model.AppError) {
-	if result := <-Srv.Store.User().GetRecentlyActiveUsersForTeam(teamId); result.Err != nil {
+	if result := <-Srv.Store.User().GetRecentlyActiveUsersForTeam(teamId, 0, 100); result.Err != nil {
 		return nil, result.Err
 	} else {
-		return result.Data.(map[string]*model.User), nil
+		users := result.Data.([]*model.User)
+		userMap := make(map[string]*model.User)
+
+		for _, user := range users {
+			userMap[user.Id] = user
+		}
+
+		return userMap, nil
 	}
 }
+
+func GetRecentlyActiveUsersForTeamPage(teamId string, page, perPage int, asAdmin bool) ([]*model.User, *model.AppError) {
+	var users []*model.User
+	if result := <-Srv.Store.User().GetRecentlyActiveUsersForTeam(teamId, page*perPage, perPage); result.Err != nil {
+		return nil, result.Err
+	} else {
+		users = result.Data.([]*model.User)
+	}
+
+	return sanitizeProfiles(users, asAdmin), nil
+}
+
+func GetNewUsersForTeamPage(teamId string, page, perPage int, asAdmin bool) ([]*model.User, *model.AppError) {
+	var users []*model.User
+	if result := <-Srv.Store.User().GetNewUsersForTeam(teamId, page*perPage, perPage); result.Err != nil {
+		return nil, result.Err
+	} else {
+		users = result.Data.([]*model.User)
+	}
+
+	return sanitizeProfiles(users, asAdmin), nil
+}
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -5875,6 +5875,10 @@
    "id": "store.sql_user.get_recently_active_users.app_error",
    "translation": "We encountered an error while finding the recently active users"
  },
+  {
+    "id": "store.sql_user.get_new_users.app_error",
+    "translation": "We encountered an error while finding the new users"
+  },
  {
    "id": "store.sql_user.get_sysadmin_profiles.app_error",
    "translation": "We encountered an error while finding user profiles"
--- a/model/client4.go
+++ b/model/client4.go
@@ -611,6 +611,28 @@ func (c *Client4) GetUsersInTeam(teamId string, page int, perPage int, etag stri
 	}
 }

+// GetNewUsersInTeam returns a page of users on a team. Page counting starts at 0.
+func (c *Client4) GetNewUsersInTeam(teamId string, page int, perPage int, etag string) ([]*User, *Response) {
+	query := fmt.Sprintf("?sort=create_at&in_team=%v&page=%v&per_page=%v", teamId, page, perPage)
+	if r, err := c.DoApiGet(c.GetUsersRoute()+query, etag); err != nil {
+		return nil, BuildErrorResponse(r, err)
+	} else {
+		defer closeBody(r)
+		return UserListFromJson(r.Body), BuildResponse(r)
+	}
+}
+
+// GetRecentlyActiveUsersInTeam returns a page of users on a team. Page counting starts at 0.
+func (c *Client4) GetRecentlyActiveUsersInTeam(teamId string, page int, perPage int, etag string) ([]*User, *Response) {
+	query := fmt.Sprintf("?sort=last_activity_at&in_team=%v&page=%v&per_page=%v", teamId, page, perPage)
+	if r, err := c.DoApiGet(c.GetUsersRoute()+query, etag); err != nil {
+		return nil, BuildErrorResponse(r, err)
+	} else {
+		defer closeBody(r)
+		return UserListFromJson(r.Body), BuildResponse(r)
+	}
+}
+
 // GetUsersNotInTeam returns a page of users who are not in a team. Page counting starts at 0.
 func (c *Client4) GetUsersNotInTeam(teamId string, page int, perPage int, etag string) ([]*User, *Response) {
 	query := fmt.Sprintf("?not_in_team=%v&page=%v&per_page=%v", teamId, page, perPage)
--- a/store/sql_user_store.go
+++ b/store/sql_user_store.go
@@ -756,7 +756,7 @@ type UserWithLastActivityAt struct {
 	LastActivityAt int64
 }

-func (us SqlUserStore) GetRecentlyActiveUsersForTeam(teamId string) StoreChannel {
+func (us SqlUserStore) GetRecentlyActiveUsersForTeam(teamId string, offset, limit int) StoreChannel {

 	storeChannel := make(StoreChannel)

@@ -774,21 +774,55 @@ func (us SqlUserStore) GetRecentlyActiveUsersForTeam(teamId string) StoreChannel
                INNER JOIN Status AS s ON s.UserId = t.UserId
            WHERE t.TeamId = :TeamId
            ORDER BY s.LastActivityAt DESC
-            LIMIT 100
-            `, map[string]interface{}{"TeamId": teamId}); err != nil {
-			result.Err = model.NewLocAppError("SqlUserStore.GetRecentlyActiveUsers", "store.sql_user.get_recently_active_users.app_error", nil, err.Error())
+            LIMIT :Limit OFFSET :Offset
+            `, map[string]interface{}{"TeamId": teamId, "Offset": offset, "Limit": limit}); err != nil {
+			result.Err = model.NewAppError("SqlUserStore.GetRecentlyActiveUsers", "store.sql_user.get_recently_active_users.app_error", nil, err.Error(), http.StatusInternalServerError)
 		} else {

-			userMap := make(map[string]*model.User)
+			userList := []*model.User{}

 			for _, userWithLastActivityAt := range users {
 				u := userWithLastActivityAt.User
 				u.Sanitize(map[string]bool{})
 				u.LastActivityAt = userWithLastActivityAt.LastActivityAt
-				userMap[u.Id] = &u
+				userList = append(userList, &u)
 			}

-			result.Data = userMap
+			result.Data = userList
+		}
+
+		storeChannel <- result
+		close(storeChannel)
+	}()
+
+	return storeChannel
+}
+
+func (us SqlUserStore) GetNewUsersForTeam(teamId string, offset, limit int) StoreChannel {
+
+	storeChannel := make(StoreChannel)
+
+	go func() {
+		result := StoreResult{}
+
+		var users []*model.User
+
+		if _, err := us.GetReplica().Select(&users, `
+            SELECT
+                u.*
+            FROM Users AS u
+                INNER JOIN TeamMembers AS t ON u.Id = t.UserId
+            WHERE t.TeamId = :TeamId
+            ORDER BY u.CreateAt DESC
+            LIMIT :Limit OFFSET :Offset
+            `, map[string]interface{}{"TeamId": teamId, "Offset": offset, "Limit": limit}); err != nil {
+			result.Err = model.NewAppError("SqlUserStore.GetNewUsersForTeam", "store.sql_user.get_new_users.app_error", nil, err.Error(), http.StatusInternalServerError)
+		} else {
+			for _, u := range users {
+				u.Sanitize(map[string]bool{})
+			}
+
+			result.Data = users
 		}

 		storeChannel <- result
--- a/store/sql_user_store_test.go
+++ b/store/sql_user_store_test.go
@@ -1291,7 +1291,22 @@ func TestUserStoreGetRecentlyActiveUsersForTeam(t *testing.T) {
 	tid := model.NewId()
 	Must(store.Team().SaveMember(&model.TeamMember{TeamId: tid, UserId: u1.Id}))

-	if r1 := <-store.User().GetRecentlyActiveUsersForTeam(tid); r1.Err != nil {
+	if r1 := <-store.User().GetRecentlyActiveUsersForTeam(tid, 0, 100); r1.Err != nil {
+		t.Fatal(r1.Err)
+	}
+}
+
+func TestUserStoreGetNewUsersForTeam(t *testing.T) {
+	Setup()
+
+	u1 := &model.User{}
+	u1.Email = model.NewId()
+	Must(store.User().Save(u1))
+	Must(store.Status().SaveOrUpdate(&model.Status{UserId: u1.Id, Status: model.STATUS_ONLINE, Manual: false, LastActivityAt: model.GetMillis(), ActiveChannel: ""}))
+	tid := model.NewId()
+	Must(store.Team().SaveMember(&model.TeamMember{TeamId: tid, UserId: u1.Id}))
+
+	if r1 := <-store.User().GetNewUsersForTeam(tid, 0, 100); r1.Err != nil {
 		t.Fatal(r1.Err)
 	}
 }
--- a/store/store.go
+++ b/store/store.go
@@ -208,7 +208,8 @@ type UserStore interface {
 	AnalyticsActiveCount(time int64) StoreChannel
 	GetUnreadCount(userId string) StoreChannel
 	GetUnreadCountForChannel(userId string, channelId string) StoreChannel
-	GetRecentlyActiveUsersForTeam(teamId string) StoreChannel
+	GetRecentlyActiveUsersForTeam(teamId string, offset, limit int) StoreChannel
+	GetNewUsersForTeam(teamId string, offset, limit int) StoreChannel
 	Search(teamId string, term string, options map[string]bool) StoreChannel
 	SearchNotInTeam(notInTeamId string, term string, options map[string]bool) StoreChannel
 	SearchInChannel(channelId string, term string, options map[string]bool) StoreChannel