PLT-3736 Fixed duplicated create_direct api calls not returning the existing channel (#4745)

* Fixed duplicated create_direct api calls not returning the existing channel

* Added unit tests for duplicated create_direct api calls

api/channel_test.go

@@ -170,9 +170,11 @@ func TestCreateDirectChannel(t *testing.T) {
 	user := th.BasicUser
 	user2 := th.BasicUser2
 
-	rchannel, err := Client.CreateDirectChannel(th.BasicUser2.Id)
+	var channel *model.Channel
-	if err != nil {
+	if result, err := Client.CreateDirectChannel(th.BasicUser2.Id); err != nil {
 		t.Fatal(err)
+	} else {
+		channel = result.Data.(*model.Channel)
 	}
 
 	channelName := ""
@@ -182,17 +184,19 @@ func TestCreateDirectChannel(t *testing.T) {
 		channelName = user2.Id + "__" + user.Id
 	}
 
-	if rchannel.Data.(*model.Channel).Name != channelName {
+	if channel.Name != channelName {
 		t.Fatal("channel name didn't match")
 	}
 
-	if rchannel.Data.(*model.Channel).Type != model.CHANNEL_DIRECT {
+	if channel.Type != model.CHANNEL_DIRECT {
 		t.Fatal("channel type was not direct")
 	}
 
-	// don't fail on direct channels already existing
+	// Don't fail on direct channels already existing and return the original channel again
-	if _, err := Client.CreateDirectChannel(th.BasicUser2.Id); err != nil {
+	if result, err := Client.CreateDirectChannel(th.BasicUser2.Id); err != nil {
 		t.Fatal(err)
+	} else if result.Data.(*model.Channel).Id != channel.Id {
+		t.Fatal("didn't return original direct channel when saving a duplicate")
 	}
 
 	if _, err := Client.CreateDirectChannel("junk"); err == nil {
@@ -202,7 +206,6 @@ func TestCreateDirectChannel(t *testing.T) {
 	if _, err := Client.CreateDirectChannel("12345678901234567890123456"); err == nil {
 		t.Fatal("should have failed with non-existent user")
 	}
-
 }
 
 func TestUpdateChannel(t *testing.T) {

store/sql_channel_store.go

@@ -210,7 +210,7 @@ func (s SqlChannelStore) saveChannelT(transaction *gorp.Transaction, channel *mo
 	if err := transaction.Insert(channel); err != nil {
 		if IsUniqueConstraintError(err.Error(), []string{"Name", "channels_name_teamid_key"}) {
 			dupChannel := model.Channel{}
-			s.GetMaster().SelectOne(&dupChannel, "SELECT * FROM Channels WHERE TeamId = :TeamId AND Name = :Name AND DeleteAt > 0", map[string]interface{}{"TeamId": channel.TeamId, "Name": channel.Name})
+			s.GetMaster().SelectOne(&dupChannel, "SELECT * FROM Channels WHERE TeamId = :TeamId AND Name = :Name", map[string]interface{}{"TeamId": channel.TeamId, "Name": channel.Name})
 			if dupChannel.DeleteAt > 0 {
 				result.Err = model.NewLocAppError("SqlChannelStore.Save", "store.sql_channel.save_channel.previously.app_error", nil, "id="+channel.Id+", "+err.Error())
 			} else {

store/sql_channel_store_test.go

@@ -88,6 +88,23 @@ func TestChannelStoreSaveDirectChannel(t *testing.T) {
 		t.Fatal("shouldn't be able to update from save")
 	}
 
+	// Attempt to save a direct channel that already exists
+	o1a := model.Channel{
+		TeamId:      o1.TeamId,
+		DisplayName: o1.DisplayName,
+		Name:        o1.Name,
+		Type:        o1.Type,
+	}
+
+	if result := <-store.Channel().SaveDirectChannel(&o1a, &m1, &m2); result.Err == nil {
+		t.Fatal("should've failed to save a duplicate direct channel")
+	} else if result.Err.Id != CHANNEL_EXISTS_ERROR {
+		t.Fatal("should've returned CHANNEL_EXISTS_ERROR")
+	} else if returned := result.Data.(*model.Channel); returned.Id != o1.Id {
+		t.Fatal("should've returned original channel when saving a duplicate direct channel")
+	}
+
+	// Attempt to save a non-direct channel
 	o1.Id = ""
 	o1.Name = "a" + model.NewId() + "b"
 	o1.Type = model.CHANNEL_OPEN