Increase Channel Purpose length to 250, and add channel field length handling code to the slack importer (#4458)

* Increase Channel Purpose length to 250.

This commit increases the maxmimum length of the channel purpose field
to 250, including the database migration necessary.

It also adds a method to the Slack importer to check the lengths of
channel properties before importing, and truncate them if necessary.

Fixes #4168

* Fix database migration.

api/channel_test.go

@@ -546,7 +546,7 @@ func TestUpdateChannelPurpose(t *testing.T) {
 	}
 
 	data["channel_id"] = channel1.Id
-	data["channel_purpose"] = strings.Repeat("a", 150)
+	data["channel_purpose"] = strings.Repeat("a", 350)
 	if _, err := Client.UpdateChannelPurpose(data); err == nil {
 		t.Fatal("should have errored on bad channel purpose")
 	}

api/slackimport.go

@@ -7,15 +7,17 @@ import (
 	"archive/zip"
 	"bytes"
 	"encoding/json"
-	l4g "github.com/alecthomas/log4go"
-	"github.com/mattermost/platform/model"
-	"github.com/mattermost/platform/utils"
 	"io"
 	"mime/multipart"
 	"path/filepath"
 	"regexp"
 	"strconv"
 	"strings"
+	"unicode/utf8"
+
+	l4g "github.com/alecthomas/log4go"
+	"github.com/mattermost/platform/model"
+	"github.com/mattermost/platform/utils"
 )
 
 type SlackChannel struct {
@@ -63,6 +65,14 @@ type SlackAttachment struct {
 	Fields  []map[string]interface{} `json:"fields"`
 }
 
+func truncateRunes(s string, i int) string {
+	runes := []rune(s)
+	if len(runes) > i {
+		return string(runes[:i])
+	}
+	return s
+}
+
 func SlackConvertTimeStamp(ts string) int64 {
 	timeString := strings.SplitN(ts, ".", 2)[0]
 
@@ -368,6 +378,30 @@ func addSlackUsersToChannel(members []string, users map[string]*model.User, chan
 	}
 }
 
+func SlackSanitiseChannelProperties(channel model.Channel) model.Channel {
+	if utf8.RuneCountInString(channel.DisplayName) > model.CHANNEL_DISPLAY_NAME_MAX_RUNES {
+		l4g.Warn("api.slackimport.slack_sanitise_channel_properties.display_name_too_long.warn", map[string]interface{}{"ChannelName": channel.DisplayName})
+		channel.DisplayName = truncateRunes(channel.DisplayName, model.CHANNEL_DISPLAY_NAME_MAX_RUNES)
+	}
+
+	if len(channel.Name) > model.CHANNEL_NAME_MAX_LENGTH {
+		l4g.Warn("api.slackimport.slack_sanitise_channel_properties.name_too_long.warn", map[string]interface{}{"ChannelName": channel.DisplayName})
+		channel.Name = channel.Name[0:model.CHANNEL_NAME_MAX_LENGTH]
+	}
+
+	if utf8.RuneCountInString(channel.Purpose) > model.CHANNEL_PURPOSE_MAX_RUNES {
+		l4g.Warn("api.slackimport.slack_sanitise_channel_properties.purpose_too_long.warn", map[string]interface{}{"ChannelName": channel.DisplayName})
+		channel.Purpose = truncateRunes(channel.Purpose, model.CHANNEL_PURPOSE_MAX_RUNES)
+	}
+
+	if utf8.RuneCountInString(channel.Header) > model.CHANNEL_HEADER_MAX_RUNES {
+		l4g.Warn("api.slackimport.slack_sanitise_channel_properties.header_too_long.warn", map[string]interface{}{"ChannelName": channel.DisplayName})
+		channel.Header = truncateRunes(channel.Header, model.CHANNEL_HEADER_MAX_RUNES)
+	}
+
+	return channel
+}
+
 func SlackAddChannels(teamId string, slackchannels []SlackChannel, posts map[string][]SlackPost, users map[string]*model.User, uploads map[string]*zip.File, botUser *model.User, log *bytes.Buffer) map[string]*model.Channel {
 	// Write Header
 	log.WriteString(utils.T("api.slackimport.slack_add_channels.added"))
@@ -383,6 +417,7 @@ func SlackAddChannels(teamId string, slackchannels []SlackChannel, posts map[str
 			Purpose:     sChannel.Purpose["value"],
 			Header:      sChannel.Topic["value"],
 		}
+		newChannel = SlackSanitiseChannelProperties(newChannel)
 		mChannel := ImportChannel(&newChannel)
 		if mChannel == nil {
 			// Maybe it already exists?

api/slackimport_test.go

@@ -4,7 +4,9 @@
 package api
 
 import (
+	"github.com/mattermost/platform/model"
 	"os"
+	"strings"
 	"testing"
 )
 
@@ -177,3 +179,41 @@ func TestSlackParsePosts(t *testing.T) {
 		t.Fatalf("Unexpected number of posts: %v", len(posts))
 	}
 }
+
+func TestSlackSanitiseChannelProperties(t *testing.T) {
+	c1 := model.Channel{
+		DisplayName: "display-name",
+		Name:        "name",
+		Purpose:     "The channel purpose",
+		Header:      "The channel header",
+	}
+
+	c1s := SlackSanitiseChannelProperties(c1)
+	if c1.DisplayName != c1s.DisplayName || c1.Name != c1s.Name || c1.Purpose != c1s.Purpose || c1.Header != c1s.Header {
+		t.Fatalf("Unexpected alterations to the channel properties.")
+	}
+
+	c2 := model.Channel{
+		DisplayName: strings.Repeat("abcdefghij", 7),
+		Name:        strings.Repeat("abcdefghij", 7),
+		Purpose:     strings.Repeat("0123456789", 30),
+		Header:      strings.Repeat("0123456789", 120),
+	}
+
+	c2s := SlackSanitiseChannelProperties(c2)
+	if c2s.DisplayName != strings.Repeat("abcdefghij", 6)+"abcd" {
+		t.Fatalf("Unexpected alterations to the channel properties: %v", c2s.DisplayName)
+	}
+
+	if c2s.Name != strings.Repeat("abcdefghij", 6)+"abcd" {
+		t.Fatalf("Unexpected alterations to the channel properties: %v", c2s.Name)
+	}
+
+	if c2s.Purpose != strings.Repeat("0123456789", 25) {
+		t.Fatalf("Unexpected alterations to the channel properties: %v", c2s.Purpose)
+	}
+
+	if c2s.Header != strings.Repeat("0123456789", 102)+"0123" {
+		t.Fatalf("Unexpected alterations to the channel properties: %v", c2s.Header)
+	}
+}

i18n/en.json

@@ -1671,6 +1671,22 @@
     "id": "api.slackimport.slack_parse_posts.error",
     "translation": "Error parsing some slack posts. Import may work anyway."
   },
+  {
+    "id": "api.slackimport.slack_sanitise_channel_properties.display_name_too_long.warn",
+    "translation": "Slack Importer: Channel {{.ChannelName}} has a display name which is too long. It will be truncated when imported."
+  },
+  {
+    "id": "api.slackimport.slack_sanitise_channel_properties.header_too_long.warn",
+    "translation": "Slack Importer: Channel {{.ChannelName}} has a header which is too long. It will be truncated when imported."
+  },
+  {
+    "id": "api.slackimport.slack_sanitise_channel_properties.name_too_long.warn",
+    "translation": "Slack Importer: Channel {{.ChannelName}} has a name which is too long. It will be truncated when imported."
+  },
+  {
+    "id": "api.slackimport.slack_sanitise_channel_properties.purpose_too_long.warn",
+    "translation": "Slack Importer: Channel {{.ChannelName}} has a purpose which is too long. It will be truncated when imported."
+  },
   {
     "id": "api.status.init.debug",
     "translation": "Initializing status api routes"

model/channel.go

@@ -10,10 +10,14 @@ import (
 )
 
 const (
-	CHANNEL_OPEN    = "O"
+	CHANNEL_OPEN                   = "O"
-	CHANNEL_PRIVATE = "P"
+	CHANNEL_PRIVATE                = "P"
-	CHANNEL_DIRECT  = "D"
+	CHANNEL_DIRECT                 = "D"
-	DEFAULT_CHANNEL = "town-square"
+	DEFAULT_CHANNEL                = "town-square"
+	CHANNEL_DISPLAY_NAME_MAX_RUNES = 64
+	CHANNEL_NAME_MAX_LENGTH        = 64
+	CHANNEL_HEADER_MAX_RUNES       = 1024
+	CHANNEL_PURPOSE_MAX_RUNES      = 250
 )
 
 type Channel struct {
@@ -75,11 +79,11 @@ func (o *Channel) IsValid() *AppError {
 		return NewLocAppError("Channel.IsValid", "model.channel.is_valid.update_at.app_error", nil, "id="+o.Id)
 	}
 
-	if utf8.RuneCountInString(o.DisplayName) > 64 {
+	if utf8.RuneCountInString(o.DisplayName) > CHANNEL_DISPLAY_NAME_MAX_RUNES {
 		return NewLocAppError("Channel.IsValid", "model.channel.is_valid.display_name.app_error", nil, "id="+o.Id)
 	}
 
-	if len(o.Name) > 64 {
+	if len(o.Name) > CHANNEL_NAME_MAX_LENGTH {
 		return NewLocAppError("Channel.IsValid", "model.channel.is_valid.name.app_error", nil, "id="+o.Id)
 	}
 
@@ -91,11 +95,11 @@ func (o *Channel) IsValid() *AppError {
 		return NewLocAppError("Channel.IsValid", "model.channel.is_valid.type.app_error", nil, "id="+o.Id)
 	}
 
-	if utf8.RuneCountInString(o.Header) > 1024 {
+	if utf8.RuneCountInString(o.Header) > CHANNEL_HEADER_MAX_RUNES {
 		return NewLocAppError("Channel.IsValid", "model.channel.is_valid.header.app_error", nil, "id="+o.Id)
 	}
 
-	if utf8.RuneCountInString(o.Purpose) > 128 {
+	if utf8.RuneCountInString(o.Purpose) > CHANNEL_PURPOSE_MAX_RUNES {
 		return NewLocAppError("Channel.IsValid", "model.channel.is_valid.purpose.app_error", nil, "id="+o.Id)
 	}
 

model/channel_test.go

@@ -78,7 +78,7 @@ func TestChannelIsValid(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	o.Purpose = strings.Repeat("01234567890", 20)
+	o.Purpose = strings.Repeat("01234567890", 30)
 	if err := o.IsValid(); err == nil {
 		t.Fatal("should be invalid")
 	}
@@ -87,6 +87,11 @@ func TestChannelIsValid(t *testing.T) {
 	if err := o.IsValid(); err != nil {
 		t.Fatal(err)
 	}
+
+	o.Purpose = strings.Repeat("0123456789", 25)
+	if err := o.IsValid(); err != nil {
+		t.Fatal(err)
+	}
 }
 
 func TestChannelPreSave(t *testing.T) {

store/sql_channel_store.go

@@ -38,7 +38,7 @@ func NewSqlChannelStore(sqlStore *SqlStore) ChannelStore {
 		table.ColMap("Name").SetMaxSize(64)
 		table.SetUniqueTogether("Name", "TeamId")
 		table.ColMap("Header").SetMaxSize(1024)
-		table.ColMap("Purpose").SetMaxSize(128)
+		table.ColMap("Purpose").SetMaxSize(250)
 		table.ColMap("CreatorId").SetMaxSize(26)
 
 		tablem := db.AddTableWithName(model.ChannelMember{}, "ChannelMembers").SetKeys(false, "ChannelId", "UserId")

store/sql_upgrade.go

@@ -201,6 +201,11 @@ func UpgradeDatabaseToVersion35(sqlStore *SqlStore) {
 	// The rest of the migration from Filenames -> FileIds is done lazily in api.GetFileInfosForPost
 	sqlStore.CreateColumnIfNotExists("Posts", "FileIds", "varchar(150)", "varchar(150)", "[]")
 
+	// Increase maximum length of the Channel table Purpose column.
+	if sqlStore.GetMaxLengthOfColumnIfExists("Channels", "Purpose") != "250" {
+		sqlStore.AlterColumnTypeIfExists("Channels", "Purpose", "varchar(250)", "varchar(250)")
+	}
+
 	// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 	// UNCOMMENT WHEN WE DO RELEASE
 	// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

webapp/components/edit_channel_purpose_modal.jsx

@@ -155,7 +155,7 @@ export default class EditChannelPurposeModal extends React.Component {
                         ref='purpose'
                         className='form-control no-resize'
                         rows='6'
-                        maxLength='128'
+                        maxLength='250'
                         defaultValue={this.props.channel.purpose}
                         onKeyDown={this.handleKeyDown}
                     />

webapp/components/new_channel_modal.jsx

@@ -283,7 +283,7 @@ class NewChannelModal extends React.Component {
                                         ref='channel_purpose'
                                         rows='4'
                                         placeholder={this.props.intl.formatMessage({id: 'channel_modal.purpose'})}
-                                        maxLength='128'
+                                        maxLength='250'
                                         value={this.props.channelData.purpose}
                                         onChange={this.handleChange}
                                         tabIndex='2'