Merge pull request #1824 from hmhealey/plt1525

PLT-1525 Fixed ChannelInviteModal displaying some users that are already in the channel

3 files changed, 61 insertions, 10 deletions

diff --git a/api/channel.go b/api/channel.go
index b85de3071..674293e19 100644
--- a/api/channel.go
+++ b/api/channel.go
@@ -9,9 +9,14 @@ import (
 	"github.com/gorilla/mux"
 	"github.com/mattermost/platform/model"
 	"net/http"
+	"strconv"
 	"strings"
 )
 
+const (
+	defaultExtraMemberLimit = 100
+)
+
 func InitChannel(r *mux.Router) {
 	l4g.Debug("Initializing channel api routes")
 
@@ -27,6 +32,7 @@ func InitChannel(r *mux.Router) {
 	sr.Handle("/update_notify_props", ApiUserRequired(updateNotifyProps)).Methods("POST")
 	sr.Handle("/{id:[A-Za-z0-9]+}/", ApiUserRequiredActivity(getChannel, false)).Methods("GET")
 	sr.Handle("/{id:[A-Za-z0-9]+}/extra_info", ApiUserRequired(getChannelExtraInfo)).Methods("GET")
+	sr.Handle("/{id:[A-Za-z0-9]+}/extra_info/{member_limit:-?[0-9]+}", ApiUserRequired(getChannelExtraInfo)).Methods("GET")
 	sr.Handle("/{id:[A-Za-z0-9]+}/join", ApiUserRequired(join)).Methods("POST")
 	sr.Handle("/{id:[A-Za-z0-9]+}/leave", ApiUserRequired(leave)).Methods("POST")
 	sr.Handle("/{id:[A-Za-z0-9]+}/delete", ApiUserRequired(deleteChannel)).Methods("POST")
@@ -730,10 +736,19 @@ func getChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 }
 
 func getChannelExtraInfo(c *Context, w http.ResponseWriter, r *http.Request) {
-
 	params := mux.Vars(r)
 	id := params["id"]
 
+	var memberLimit int
+	if memberLimitString, ok := params["member_limit"]; !ok {
+		memberLimit = defaultExtraMemberLimit
+	} else if memberLimitInt64, err := strconv.ParseInt(memberLimitString, 10, 0); err != nil {
+		c.Err = model.NewAppError("getChannelExtraInfo", "Failed to parse member limit", err.Error())
+		return
+	} else {
+		memberLimit = int(memberLimitInt64)
+	}
+
 	sc := Srv.Store.Channel().Get(id)
 	var channel *model.Channel
 	if cresult := <-sc; cresult.Err != nil {
@@ -743,13 +758,13 @@ func getChannelExtraInfo(c *Context, w http.ResponseWriter, r *http.Request) {
 		channel = cresult.Data.(*model.Channel)
 	}
 
-	extraEtag := channel.ExtraEtag()
+	extraEtag := channel.ExtraEtag(memberLimit)
 	if HandleEtag(extraEtag, w, r) {
 		return
 	}
 
 	scm := Srv.Store.Channel().GetMember(id, c.Session.UserId)
-	ecm := Srv.Store.Channel().GetExtraMembers(id, 100)
+	ecm := Srv.Store.Channel().GetExtraMembers(id, memberLimit)
 	ccm := Srv.Store.Channel().GetMemberCount(id)
 
 	if cmresult := <-scm; cmresult.Err != nil {
diff --git a/api/channel_benchmark_test.go b/api/channel_benchmark_test.go
index fb8dd61bc..d6e1e5a55 100644
--- a/api/channel_benchmark_test.go
+++ b/api/channel_benchmark_test.go
@@ -189,7 +189,7 @@ func BenchmarkGetChannelExtraInfo(b *testing.B) {
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
 		for j := range channels {
-			Client.Must(Client.GetChannelExtraInfo(channels[j].Id, ""))
+			Client.Must(Client.GetChannelExtraInfo(channels[j].Id, -1, ""))
 		}
 	}
 }
diff --git a/api/channel_test.go b/api/channel_test.go
index 4ef164cba..117278378 100644
--- a/api/channel_test.go
+++ b/api/channel_test.go
@@ -674,7 +674,7 @@ func TestGetChannelExtraInfo(t *testing.T) {
 	channel1 := &model.Channel{DisplayName: "A Test API Name", Name: "a" + model.NewId() + "a", Type: model.CHANNEL_OPEN, TeamId: team.Id}
 	channel1 = Client.Must(Client.CreateChannel(channel1)).Data.(*model.Channel)
 
-	rget := Client.Must(Client.GetChannelExtraInfo(channel1.Id, ""))
+	rget := Client.Must(Client.GetChannelExtraInfo(channel1.Id, -1, ""))
 	data := rget.Data.(*model.ChannelExtra)
 	if data.Id != channel1.Id {
 		t.Fatal("couldnt't get extra info")
@@ -690,7 +690,7 @@ func TestGetChannelExtraInfo(t *testing.T) {
 
 	currentEtag := rget.Etag
 
-	if cache_result, err := Client.GetChannelExtraInfo(channel1.Id, currentEtag); err != nil {
+	if cache_result, err := Client.GetChannelExtraInfo(channel1.Id, -1, currentEtag); err != nil {
 		t.Fatal(err)
 	} else if cache_result.Data.(*model.ChannelExtra) != nil {
 		t.Log(cache_result.Data)
@@ -708,7 +708,7 @@ func TestGetChannelExtraInfo(t *testing.T) {
 	Client2.LoginByEmail(team.Name, user2.Email, "pwd")
 	Client2.Must(Client2.JoinChannel(channel1.Id))
 
-	if cache_result, err := Client.GetChannelExtraInfo(channel1.Id, currentEtag); err != nil {
+	if cache_result, err := Client.GetChannelExtraInfo(channel1.Id, -1, currentEtag); err != nil {
 		t.Fatal(err)
 	} else if cache_result.Data.(*model.ChannelExtra) == nil {
 		t.Log(cache_result.Data)
@@ -717,7 +717,7 @@ func TestGetChannelExtraInfo(t *testing.T) {
 		currentEtag = cache_result.Etag
 	}
 
-	if cache_result, err := Client.GetChannelExtraInfo(channel1.Id, currentEtag); err != nil {
+	if cache_result, err := Client.GetChannelExtraInfo(channel1.Id, -1, currentEtag); err != nil {
 		t.Fatal(err)
 	} else if cache_result.Data.(*model.ChannelExtra) != nil {
 		t.Log(cache_result.Data)
@@ -728,7 +728,7 @@ func TestGetChannelExtraInfo(t *testing.T) {
 
 	Client2.Must(Client2.LeaveChannel(channel1.Id))
 
-	if cache_result, err := Client.GetChannelExtraInfo(channel1.Id, currentEtag); err != nil {
+	if cache_result, err := Client.GetChannelExtraInfo(channel1.Id, -1, currentEtag); err != nil {
 		t.Fatal(err)
 	} else if cache_result.Data.(*model.ChannelExtra) == nil {
 		t.Log(cache_result.Data)
@@ -737,7 +737,7 @@ func TestGetChannelExtraInfo(t *testing.T) {
 		currentEtag = cache_result.Etag
 	}
 
-	if cache_result, err := Client.GetChannelExtraInfo(channel1.Id, currentEtag); err != nil {
+	if cache_result, err := Client.GetChannelExtraInfo(channel1.Id, -1, currentEtag); err != nil {
 		t.Fatal(err)
 	} else if cache_result.Data.(*model.ChannelExtra) != nil {
 		t.Log(cache_result.Data)
@@ -745,6 +745,42 @@ func TestGetChannelExtraInfo(t *testing.T) {
 	} else {
 		currentEtag = cache_result.Etag
 	}
+
+	Client2.Must(Client2.JoinChannel(channel1.Id))
+
+	if cache_result, err := Client.GetChannelExtraInfo(channel1.Id, 2, currentEtag); err != nil {
+		t.Fatal(err)
+	} else if extra := cache_result.Data.(*model.ChannelExtra); extra == nil {
+		t.Fatal("response should not be empty")
+	} else if len(extra.Members) != 2 {
+		t.Fatal("should've returned 2 members")
+	} else if extra.MemberCount != 2 {
+		t.Fatal("should've returned member count of 2")
+	} else {
+		currentEtag = cache_result.Etag
+	}
+
+	if cache_result, err := Client.GetChannelExtraInfo(channel1.Id, 1, currentEtag); err != nil {
+		t.Fatal(err)
+	} else if extra := cache_result.Data.(*model.ChannelExtra); extra == nil {
+		t.Fatal("response should not be empty")
+	} else if len(extra.Members) != 1 {
+		t.Fatal("should've returned only 1 member")
+	} else if extra.MemberCount != 2 {
+		t.Fatal("should've returned member count of 2")
+	} else {
+		currentEtag = cache_result.Etag
+	}
+
+	if cache_result, err := Client.GetChannelExtraInfo(channel1.Id, 1, currentEtag); err != nil {
+		t.Fatal(err)
+	} else if cache_result.Data.(*model.ChannelExtra) != nil {
+		t.Log(cache_result.Data)
+		t.Fatal("response should be empty")
+	} else {
+		currentEtag = cache_result.Etag
+	}
+
 }
 
 func TestAddChannelMember(t *testing.T) {