Don't return error if already part of channel being joined (#2814)

6 files changed, 32 insertions, 10 deletions

diff --git a/Makefile b/Makefile
index 0d22d392d..f1300176b 100644
--- a/Makefile
+++ b/Makefile
@@ -143,7 +143,7 @@ check-style:
 test: start-docker
 	@echo Running tests
 
-	$(GO) test $(GOFLAGS) -run=$(TESTS) -test.v -test.timeout=240s ./api || exit 1
+	$(GO) test $(GOFLAGS) -run=$(TESTS) -test.v -test.timeout=340s ./api || exit 1
 	$(GO) test $(GOFLAGS) -run=$(TESTS) -test.v -test.timeout=12s ./model || exit 1
 	$(GO) test $(GOFLAGS) -run=$(TESTS) -test.v -test.timeout=120s ./store || exit 1
 	$(GO) test $(GOFLAGS) -run=$(TESTS) -test.v -test.timeout=120s ./utils || exit 1
diff --git a/api/channel.go b/api/channel.go
index 4b0e99b20..5f0d03246 100644
--- a/api/channel.go
+++ b/api/channel.go
@@ -513,9 +513,18 @@ func AddUserToChannel(user *model.User, channel *model.Channel) (*model.ChannelM
 		return nil, model.NewLocAppError("AddUserToChannel", "api.channel.add_user_to_channel.type.app_error", nil, "")
 	}
 
+	if result := <-Srv.Store.Channel().GetMember(channel.Id, user.Id); result.Err != nil {
+		if result.Err.Id != store.MISSING_MEMBER_ERROR {
+			return nil, result.Err
+		}
+	} else {
+		channelMember := result.Data.(model.ChannelMember)
+		return &channelMember, nil
+	}
+
 	newMember := &model.ChannelMember{ChannelId: channel.Id, UserId: user.Id, NotifyProps: model.GetDefaultChannelNotifyProps()}
-	if cmresult := <-Srv.Store.Channel().SaveMember(newMember); cmresult.Err != nil {
-		l4g.Error("Failed to add member user_id=%v channel_id=%v err=%v", user.Id, channel.Id, cmresult.Err)
+	if result := <-Srv.Store.Channel().SaveMember(newMember); result.Err != nil {
+		l4g.Error("Failed to add member user_id=%v channel_id=%v err=%v", user.Id, channel.Id, result.Err)
 		return nil, model.NewLocAppError("AddUserToChannel", "api.channel.add_user.to.channel.failed.app_error", nil, "")
 	}
 
diff --git a/api/channel_test.go b/api/channel_test.go
index 8ac785f77..a70e5532a 100644
--- a/api/channel_test.go
+++ b/api/channel_test.go
@@ -691,8 +691,8 @@ func TestAddChannelMember(t *testing.T) {
 		t.Fatal("Should have errored, bad user id")
 	}
 
-	if _, err := Client.AddChannelMember(channel1.Id, user2.Id); err == nil {
-		t.Fatal("Should have errored, user already a member")
+	if _, err := Client.AddChannelMember(channel1.Id, user2.Id); err != nil {
+		t.Fatal(err)
 	}
 
 	if _, err := Client.AddChannelMember("sgdsgsdg", user2.Id); err == nil {
diff --git a/i18n/en.json b/i18n/en.json
index 4b67d8c78..e8e20a12a 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -2852,6 +2852,10 @@
     "translation": "We couldn't get all the channels"
   },
   {
+    "id": "store.sql_channel.get_member.missing.app_error",
+    "translation": "No channel member found for that user id and channel id"
+  },
+  {
     "id": "store.sql_channel.get_member.app_error",
     "translation": "We couldn't get the channel member"
   },
diff --git a/store/sql_channel_store.go b/store/sql_channel_store.go
index 46f56e7eb..6d549c7f0 100644
--- a/store/sql_channel_store.go
+++ b/store/sql_channel_store.go
@@ -12,6 +12,7 @@ import (
 
 const (
 	MISSING_CHANNEL_ERROR = "store.sql_channel.get_by_name.missing.app_error"
+	MISSING_MEMBER_ERROR  = "store.sql_channel.get_member.missing.app_error"
 )
 
 type SqlChannelStore struct {
@@ -572,9 +573,13 @@ func (s SqlChannelStore) GetMember(channelId string, userId string) StoreChannel
 		result := StoreResult{}
 
 		var member model.ChannelMember
-		err := s.GetReplica().SelectOne(&member, "SELECT * FROM ChannelMembers WHERE ChannelId = :ChannelId AND UserId = :UserId", map[string]interface{}{"ChannelId": channelId, "UserId": userId})
-		if err != nil {
-			result.Err = model.NewLocAppError("SqlChannelStore.GetMember", "store.sql_channel.get_member.app_error", nil, "channel_id="+channelId+"user_id="+userId+","+err.Error())
+
+		if err := s.GetReplica().SelectOne(&member, "SELECT * FROM ChannelMembers WHERE ChannelId = :ChannelId AND UserId = :UserId", map[string]interface{}{"ChannelId": channelId, "UserId": userId}); err != nil {
+			if err == sql.ErrNoRows {
+				result.Err = model.NewLocAppError("SqlChannelStore.GetMember", MISSING_MEMBER_ERROR, nil, "channel_id="+channelId+"user_id="+userId+","+err.Error())
+			} else {
+				result.Err = model.NewLocAppError("SqlChannelStore.GetMember", "store.sql_channel.get_member.app_error", nil, "channel_id="+channelId+"user_id="+userId+","+err.Error())
+			}
 		} else {
 			result.Data = member
 		}
diff --git a/webapp/components/notify_counts.jsx b/webapp/components/notify_counts.jsx
index 9238c8736..8f9eadab7 100644
--- a/webapp/components/notify_counts.jsx
+++ b/webapp/components/notify_counts.jsx
@@ -9,8 +9,12 @@ function getCountsStateFromStores() {
     var channels = ChannelStore.getAll();
     var members = ChannelStore.getAllMembers();
 
-    channels.forEach(function setChannelInfo(channel) {
+    channels.forEach((channel) => {
         var channelMember = members[channel.id];
+        if (channelMember == null) {
+            return;
+        }
+
         if (channel.type === 'D') {
             count += channel.total_msg_count - channelMember.msg_count;
         } else if (channelMember.mention_count > 0) {
@@ -20,7 +24,7 @@ function getCountsStateFromStores() {
         }
     });
 
-    return {count: count};
+    return {count};
 }
 
 import React from 'react';