From f7476b2fb6a01d50868a128c1d1f77c14691482d Mon Sep 17 00:00:00 2001 From: George Goldberg Date: Thu, 26 Jan 2017 02:14:12 +0000 Subject: PLT-4378 Slack import when channel name is deleted (#4649) This fixes the issue where the channel fails to Import from Slack if there is already a channel with the same name on Mattermost that has been deleted. --- app/slackimport.go | 22 ++++++++++++++++------ i18n/en.json | 8 ++++++++ store/sql_channel_store.go | 25 +++++++++++++++++++++++++ store/sql_channel_store_test.go | 33 ++++++++++++++++++++++++++++++++- store/store.go | 1 + 5 files changed, 82 insertions(+), 7 deletions(-) diff --git a/app/slackimport.go b/app/slackimport.go index 53f455069..508803126 100644 --- a/app/slackimport.go +++ b/app/slackimport.go @@ -470,18 +470,28 @@ func SlackAddChannels(teamId string, slackchannels []SlackChannel, posts map[str Header: sChannel.Topic["value"], } newChannel = SlackSanitiseChannelProperties(newChannel) - mChannel := ImportChannel(&newChannel) + + var mChannel *model.Channel + if result := <-Srv.Store.Channel().GetByName(teamId, sChannel.Name); result.Err == nil { + // The channel already exists as an active channel. Merge with the existing one. + mChannel = result.Data.(*model.Channel) + log.WriteString(utils.T("api.slackimport.slack_add_channels.merge", map[string]interface{}{"DisplayName": newChannel.DisplayName})) + } else if result := <-Srv.Store.Channel().GetDeletedByName(teamId, sChannel.Name); result.Err == nil { + // The channel already exists but has been deleted. Generate a random string for the handle instead. + newChannel.Name = model.NewId() + newChannel = SlackSanitiseChannelProperties(newChannel) + } + if mChannel == nil { - // Maybe it already exists? - if result := <-Srv.Store.Channel().GetByName(teamId, sChannel.Name); result.Err != nil { + // Haven't found an existing channel to merge with. Try importing it as a new one. + mChannel = ImportChannel(&newChannel) + if mChannel == nil { l4g.Warn(utils.T("api.slackimport.slack_add_channels.import_failed.warn"), newChannel.DisplayName) log.WriteString(utils.T("api.slackimport.slack_add_channels.import_failed", map[string]interface{}{"DisplayName": newChannel.DisplayName})) continue - } else { - mChannel = result.Data.(*model.Channel) - log.WriteString(utils.T("api.slackimport.slack_add_channels.merge", map[string]interface{}{"DisplayName": newChannel.DisplayName})) } } + addSlackUsersToChannel(sChannel.Members, users, mChannel, log) log.WriteString(newChannel.DisplayName + "\r\n") addedChannels[sChannel.Id] = mChannel diff --git a/i18n/en.json b/i18n/en.json index aca8fb9d0..6cc82bf37 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -4315,6 +4315,14 @@ "id": "store.sql_channel.get_channels.not_found.app_error", "translation": "No channels were found" }, + { + "id": "store.sql_channel.get_deleted_by_name.existing.app_error", + "translation": "We couldn't find the existing deleted channel" + }, + { + "id": "store.sql_channel.get_deleted_by_name.missing.app_error", + "translation": "No deleted channel exists with that name" + }, { "id": "store.sql_channel.get_extra_members.app_error", "translation": "We couldn't get the extra info for channel members" diff --git a/store/sql_channel_store.go b/store/sql_channel_store.go index 6ac69df5a..e3df07f74 100644 --- a/store/sql_channel_store.go +++ b/store/sql_channel_store.go @@ -579,6 +579,31 @@ func (s SqlChannelStore) getByName(teamId string, name string, includeDeleted bo return storeChannel } +func (s SqlChannelStore) GetDeletedByName(teamId string, name string) StoreChannel { + storeChannel := make(StoreChannel, 1) + + go func() { + result := StoreResult{} + + channel := model.Channel{} + + if err := s.GetReplica().SelectOne(&channel, "SELECT * FROM Channels WHERE (TeamId = :TeamId OR TeamId = '') AND Name = :Name AND DeleteAt != 0", map[string]interface{}{"TeamId": teamId, "Name": name}); err != nil { + if err == sql.ErrNoRows { + result.Err = model.NewLocAppError("SqlChannelStore.GetDeletedByName", "store.sql_channel.get_deleted_by_name.missing.app_error", nil, "teamId="+teamId+", "+"name="+name+", "+err.Error()) + } else { + result.Err = model.NewLocAppError("SqlChannelStore.GetDeletedByName", "store.sql_channel.get_deleted_by_name.existing.app_error", nil, "teamId="+teamId+", "+"name="+name+", "+err.Error()) + } + } else { + result.Data = &channel + } + + storeChannel <- result + close(storeChannel) + }() + + return storeChannel +} + func (s SqlChannelStore) SaveMember(member *model.ChannelMember) StoreChannel { storeChannel := make(StoreChannel, 1) diff --git a/store/sql_channel_store_test.go b/store/sql_channel_store_test.go index 38446bf94..5202a7c29 100644 --- a/store/sql_channel_store_test.go +++ b/store/sql_channel_store_test.go @@ -352,7 +352,8 @@ func TestChannelStoreGetByName(t *testing.T) { o1.Type = model.CHANNEL_OPEN Must(store.Channel().Save(&o1)) - if r1 := <-store.Channel().GetByName(o1.TeamId, o1.Name); r1.Err != nil { + r1 := <-store.Channel().GetByName(o1.TeamId, o1.Name) + if r1.Err != nil { t.Fatal(r1.Err) } else { if r1.Data.(*model.Channel).ToJson() != o1.ToJson() { @@ -363,6 +364,36 @@ func TestChannelStoreGetByName(t *testing.T) { if err := (<-store.Channel().GetByName(o1.TeamId, "")).Err; err == nil { t.Fatal("Missing id should have failed") } + + Must(store.Channel().Delete(r1.Data.(*model.Channel).Id, model.GetMillis())) + + if err := (<-store.Channel().GetByName(o1.TeamId, "")).Err; err == nil { + t.Fatal("Deleted channel should not be returned by GetByName()") + } +} + +func TestChannelStoreGetDeletedByName(t *testing.T) { + Setup() + + o1 := model.Channel{} + o1.TeamId = model.NewId() + o1.DisplayName = "Name" + o1.Name = "a" + model.NewId() + "b" + o1.Type = model.CHANNEL_OPEN + o1.DeleteAt = model.GetMillis() + Must(store.Channel().Save(&o1)) + + if r1 := <-store.Channel().GetDeletedByName(o1.TeamId, o1.Name); r1.Err != nil { + t.Fatal(r1.Err) + } else { + if r1.Data.(*model.Channel).ToJson() != o1.ToJson() { + t.Fatal("invalid returned channel") + } + } + + if err := (<-store.Channel().GetDeletedByName(o1.TeamId, "")).Err; err == nil { + t.Fatal("Missing id should have failed") + } } func TestChannelMemberStore(t *testing.T) { diff --git a/store/store.go b/store/store.go index 48819e2d7..cd918c033 100644 --- a/store/store.go +++ b/store/store.go @@ -93,6 +93,7 @@ type ChannelStore interface { PermanentDeleteByTeam(teamId string) StoreChannel GetByName(team_id string, name string) StoreChannel GetByNameIncludeDeleted(team_id string, name string) StoreChannel + GetDeletedByName(team_id string, name string) StoreChannel GetChannels(teamId string, userId string) StoreChannel GetMoreChannels(teamId string, userId string, offset int, limit int) StoreChannel GetChannelCounts(teamId string, userId string) StoreChannel -- cgit v1.2.3-1-g7c22