diff options
author | Harrison Healey <harrisonmhealey@gmail.com> | 2016-12-09 18:43:03 -0500 |
---|---|---|
committer | Corey Hulen <corey@hulen.com> | 2016-12-09 15:43:03 -0800 |
commit | aaa41535f8ac9656d0cd399aeedf58de9b374b65 (patch) | |
tree | 34d15671bf4b6f50fdb96a0fe1f1967885d87931 | |
parent | 9670afed82e6c3192d230f107b836e21bb0c01a6 (diff) | |
download | chat-aaa41535f8ac9656d0cd399aeedf58de9b374b65.tar.gz chat-aaa41535f8ac9656d0cd399aeedf58de9b374b65.tar.bz2 chat-aaa41535f8ac9656d0cd399aeedf58de9b374b65.zip |
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
-rw-r--r-- | api/channel_test.go | 17 | ||||
-rw-r--r-- | store/sql_channel_store.go | 2 | ||||
-rw-r--r-- | store/sql_channel_store_test.go | 17 |
3 files changed, 28 insertions, 8 deletions
diff --git a/api/channel_test.go b/api/channel_test.go index 611fa9339..5b44b6ab0 100644 --- a/api/channel_test.go +++ b/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) - if err != nil { + var channel *model.Channel + 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 - if _, err := Client.CreateDirectChannel(th.BasicUser2.Id); err != nil { + // Don't fail on direct channels already existing and return the original channel again + 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) { diff --git a/store/sql_channel_store.go b/store/sql_channel_store.go index 3ce672a68..aed568b46 100644 --- a/store/sql_channel_store.go +++ b/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 { diff --git a/store/sql_channel_store_test.go b/store/sql_channel_store_test.go index 9b77639b0..d4bcf6b26 100644 --- a/store/sql_channel_store_test.go +++ b/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 |