Merge branch 'master' into mark-as-unread

.circleci/config.yml

@@ -205,7 +205,7 @@ jobs:
             docker-compose --no-ansi exec -T postgres sh -c 'exec echo "DROP DATABASE migrated; DROP DATABASE latest;" | exec psql -U mmuser mattermost_test'
 
             echo "Generating diff"
-            diff migrated.sql latest.sql > diff.txt && echo "Both schemas are same" || (cat diff.txt && exit 1)
+            diff migrated.sql latest.sql > diff.txt && echo "Both schemas are same" || (echo "Schema mismatch" && cat diff.txt && exit 1)
           no_output_timeout: 1h
       - run:
           name: MySQL schema migration validation
@@ -234,6 +234,11 @@ jobs:
               -w /go/src/github.com/mattermost/mattermost-server \
               mattermost/mattermost-build-server:feb-28-2019 \
               bash -c 'ulimit -n 8096; make ARGS="version" run-cli && make MM_SQLSETTINGS_DATASOURCE="mmuser:mostest@tcp(mysql:3306)/latest?charset=utf8mb4,utf8&readTimeout=30s&writeTimeout=30s" ARGS="version" run-cli'
+
+            echo "Ignoring known MySQL mismatch: ChannelMembers.SchemeGuest"
+            docker-compose --no-ansi exec -T mysql mysql -D migrated -uroot -pmostest -e "ALTER TABLE ChannelMembers DROP COLUMN SchemeGuest;"
+            docker-compose --no-ansi exec -T mysql mysql -D latest -uroot -pmostest -e "ALTER TABLE ChannelMembers DROP COLUMN SchemeGuest;"
+
             echo "Generating dump"
             docker-compose --no-ansi exec -T mysql mysqldump --skip-opt --no-data --compact -u root -pmostest migrated > migrated.sql
             docker-compose --no-ansi exec -T mysql mysqldump --skip-opt --no-data --compact -u root -pmostest latest > latest.sql
@@ -242,7 +247,7 @@ jobs:
             docker-compose --no-ansi exec -T mysql mysql -uroot -pmostest -e 'DROP DATABASE migrated; DROP DATABASE latest'
 
             echo "Generating diff"
-            diff migrated.sql latest.sql > diff.txt && echo "Both schemas are same" || (cat diff.txt && exit 1)
+            diff migrated.sql latest.sql > diff.txt && echo "Both schemas are same" || (echo "Schema mismatch" && cat diff.txt && exit 1)
           no_output_timeout: 1h
   upload-s3-sha:
     docker:

api4/ldap.go

@@ -155,13 +155,21 @@ func linkLdapGroup(c *Context, w http.ResponseWriter, r *http.Request) {
 	var status int
 	var newOrUpdatedGroup *model.Group
 
+	// Truncate display name if necessary
+	var displayName string
+	if len(ldapGroup.DisplayName) > model.GroupDisplayNameMaxLength {
+		displayName = ldapGroup.DisplayName[:model.GroupDisplayNameMaxLength]
+	} else {
+		displayName = ldapGroup.DisplayName
+	}
+
 	// Group has been previously linked
 	if group != nil {
 		if group.DeleteAt == 0 {
 			newOrUpdatedGroup = group
 		} else {
 			group.DeleteAt = 0
-			group.DisplayName = ldapGroup.DisplayName
+			group.DisplayName = displayName
 			group.RemoteId = ldapGroup.RemoteId
 			newOrUpdatedGroup, err = c.App.UpdateGroup(group)
 			if err != nil {
@@ -178,7 +186,7 @@ func linkLdapGroup(c *Context, w http.ResponseWriter, r *http.Request) {
 		// the LDAP group name with an appended duplicate-breaker.
 		newGroup := &model.Group{
 			Name:        model.NewId(),
-			DisplayName: ldapGroup.DisplayName,
+			DisplayName: displayName,
 			RemoteId:    ldapGroup.RemoteId,
 			Source:      model.GroupSourceLdap,
 		}

app/export.go

@@ -5,7 +5,6 @@ package app
 
 import (
 	"encoding/json"
-	"fmt"
 	"io"
 	"net/http"
 	"os"
@@ -414,7 +413,7 @@ func (a *App) BuildPostReactions(postId string) (*[]ReactionImportData, *model.A
 		user, err = a.Srv.Store.User().Get(reaction.UserId)
 		if err != nil {
 			if err.Id == store.MISSING_ACCOUNT_ERROR { // this is a valid case, the user that reacted might've been deleted by now
-				mlog.Info(fmt.Sprintf("Skipping reactions by user %v, since the entity doesn't exist anymore", reaction.UserId))
+				mlog.Info("Skipping reactions by user since the entity doesn't exist anymore", mlog.String("user_id", reaction.UserId))
 				continue
 			}
 			return nil, err

app/import.go

@@ -6,7 +6,6 @@ package app
 import (
 	"bufio"
 	"encoding/json"
-	"fmt"
 	"io"
 	"net/http"
 	"strings"
@@ -19,7 +18,7 @@ import (
 
 func stopOnError(err LineImportWorkerError) bool {
 	if err.Error.Id == "api.file.upload_file.large_image.app_error" {
-		mlog.Warn(fmt.Sprintf("Large image import error: %s", err.Error.Error()))
+		mlog.Warn("Large image import error", mlog.Err(err.Error))
 		return false
 	}
 	return true

app/post.go

@@ -789,7 +789,7 @@ func (a *App) GetPostsForChannelAroundLastUnread(channelId, userId string, limit
 		return model.NewPostList(), nil
 	}
 
-	postList, err := a.GetPostThread(lastUnreadPostId, false)
+	postList, err := a.GetPostThread(lastUnreadPostId, skipFetchThreads)
 	if err != nil {
 		return nil, err
 	}

build/Jenkinsfile.pr

@@ -353,6 +353,10 @@ pipeline {
 				dir('src/github.com/mattermost/mattermost-server') {
 					ansiColor('xterm') {
 						sh """
+                                                echo "Ignoring known MySQL mismatch: ChannelMembers.SchemeGuest"
+                                                /usr/local/bin/docker-compose --no-ansi -f build/docker-compose.yml exec -T mysql mysql -D migrated -uroot -pmostest -e "ALTER TABLE ChannelMembers DROP COLUMN SchemeGuest;"
+                                                /usr/local/bin/docker-compose --no-ansi -f build/docker-compose.yml exec -T mysql mysql -D latest -uroot -pmostest -e "ALTER TABLE ChannelMembers DROP COLUMN SchemeGuest;"
+
 						echo "Generating dump"
 						/usr/local/bin/docker-compose --no-ansi -f build/docker-compose.yml  exec -T mysql mysqldump --skip-opt --no-data --compact -u root -pmostest migrated > migrated.sql
 						/usr/local/bin/docker-compose --no-ansi -f build/docker-compose.yml  exec -T mysql mysqldump --skip-opt --no-data --compact -u root -pmostest latest > latest.sql

model/emoji_test.go

@@ -20,9 +20,7 @@ func TestEmojiIsValid(t *testing.T) {
 		Name:      "name",
 	}
 
-	if err := emoji.IsValid(); err != nil {
+	require.Nil(t, emoji.IsValid())
-		t.Fatal(err)
-	}
 
 	emoji.Id = "1234"
 	require.NotNil(t, emoji.IsValid())

model/manifest.go

@@ -130,6 +130,10 @@ type Manifest struct {
 	// A description of what your plugin is and does.
 	Description string `json:"description,omitempty" yaml:"description,omitempty"`
 
+	// A relative file path in the bundle that points to the plugins svg icon for use with the Plugin Marketplace.
+	// This should be relative to the root of your bundle and the location of the manifest file. Bitmap image formats are not supported.
+	IconPath string `json:"icon_path,omitempty" yaml:"icon_path,omitempty"`
+
 	// A version number for your plugin. Semantic versioning is recommended: http://semver.org
 	Version string `json:"version" yaml:"version"`
 

model/manifest_test.go

@@ -64,6 +64,7 @@ func TestFindManifest(t *testing.T) {
 func TestManifestUnmarshal(t *testing.T) {
 	expected := Manifest{
 		Id:               "theid",
+		IconPath:         "assets/icon.svg",
 		MinServerVersion: "5.6.0",
 		Server: &ManifestServer{
 			Executable: "theexecutable",
@@ -102,6 +103,7 @@ func TestManifestUnmarshal(t *testing.T) {
 	var yamlResult Manifest
 	require.NoError(t, yaml.Unmarshal([]byte(`
 id: theid
+icon_path: assets/icon.svg
 min_server_version: 5.6.0
 server:
     executable: theexecutable
@@ -131,7 +133,8 @@ settings_schema:
 	var jsonResult Manifest
 	require.NoError(t, json.Unmarshal([]byte(`{
 	"id": "theid",
-  "min_server_version": "5.6.0",
+	"icon_path": "assets/icon.svg",
+	"min_server_version": "5.6.0",
 	"server": {
 		"executable": "theexecutable",
 		"executables": {

model/post_list_test.go

@@ -24,17 +24,9 @@ func TestPostListJson(t *testing.T) {
 	json := pl.ToJson()
 	rpl := PostListFromJson(strings.NewReader(json))
 
-	if rpl.Posts[p1.Id].Message != p1.Message {
+	assert.Equal(t, p1.Message, rpl.Posts[p1.Id].Message, "failed to serialize p1 message")
-		t.Fatal("failed to serialize")
+	assert.Equal(t, p2.Message, rpl.Posts[p2.Id].Message, "failed to serialize p2 message")
-	}
+	assert.Equal(t, p2.Id, rpl.Order[1], "failed to serialize p2 Id")
-
-	if rpl.Posts[p2.Id].Message != p2.Message {
-		t.Fatal("failed to serialize")
-	}
-
-	if rpl.Order[1] != p2.Id {
-		t.Fatal("failed to serialize")
-	}
 }
 
 func TestPostListExtend(t *testing.T) {

model/team_member_test.go

@@ -6,6 +6,8 @@ package model
 import (
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestTeamMemberJson(t *testing.T) {
@@ -13,22 +15,17 @@ func TestTeamMemberJson(t *testing.T) {
 	json := o.ToJson()
 	ro := TeamMemberFromJson(strings.NewReader(json))
 
-	if o.TeamId != ro.TeamId {
+	require.Equal(t, o.TeamId, ro.TeamId, "Ids do not match")
-		t.Fatal("Ids do not match")
-	}
 }
 
 func TestTeamMemberIsValid(t *testing.T) {
 	o := TeamMember{}
 
-	if err := o.IsValid(); err == nil {
+	require.Error(t, o.IsValid(), "should be invalid")
-		t.Fatal("should be invalid")
-	}
 
 	o.TeamId = NewId()
-	if err := o.IsValid(); err == nil {
+
-		t.Fatal("should be invalid")
+	require.Error(t, o.IsValid(), "should be invalid")
-	}
 
 	/*o.UserId = NewId()
 	o.Roles = "blahblah"
@@ -47,11 +44,8 @@ func TestUnreadMemberJson(t *testing.T) {
 	json := o.ToJson()
 
 	r := TeamUnreadFromJson(strings.NewReader(json))
-	if o.TeamId != r.TeamId {
-		t.Fatal("Ids do not match")
-	}
 
-	if o.MsgCount != r.MsgCount {
+	require.Equal(t, o.TeamId, r.TeamId, "Ids do not match")
-		t.Fatal("MsgCount do not match")
+
-	}
+	require.Equal(t, o.MsgCount, r.MsgCount, "MsgCount do not match")
 }

scripts/mysql-migration-test.sh

@@ -22,6 +22,10 @@ make ARGS="config set SqlSettings.DataSource 'mmuser:mostest@tcp(localhost:3306)
 echo "Setting up fresh db"
 make ARGS="version --config $TMPDIR/config.json" run-cli
 
+echo "Ignoring known MySQL mismatch: ChannelMembers.SchemeGuest"
+docker exec mattermost-mysql mysql -D migrated -uroot -pmostest -e "ALTER TABLE ChannelMembers DROP COLUMN SchemeGuest;"
+docker exec mattermost-mysql mysql -D latest -uroot -pmostest -e "ALTER TABLE ChannelMembers DROP COLUMN SchemeGuest;"
+
 echo "Generating dump"
 docker exec mattermost-mysql mysqldump --skip-opt --no-data --compact -u root -pmostest migrated > $DUMPDIR/migrated.sql
 docker exec mattermost-mysql mysqldump --skip-opt --no-data --compact -u root -pmostest latest > $DUMPDIR/latest.sql
@@ -33,7 +37,12 @@ echo "Generating diff"
 diff $DUMPDIR/migrated.sql $DUMPDIR/latest.sql > $DUMPDIR/diff.txt
 diffErrorCode=$?
 
-if [ $diffErrorCode -eq 0 ]; then echo "Both schemas are same";else cat $DUMPDIR/diff.txt; fi
+if [ $diffErrorCode -eq 0 ]; then
+    echo "Both schemas are same"
+else
+    echo "Schema mismatch"
+    cat $DUMPDIR/diff.txt
+fi
 rm -rf $TMPDIR $DUMPDIR
 
 exit $diffErrorCode

scripts/psql-migration-test.sh

@@ -33,7 +33,12 @@ echo "Generating diff"
 diff $DUMPDIR/migrated.sql $DUMPDIR/latest.sql > $DUMPDIR/diff.txt
 diffErrorCode=$?
 
-if [ $diffErrorCode -eq 0 ]; then echo "Both schemas are same";else cat $DUMPDIR/diff.txt; fi
+if [ $diffErrorCode -eq 0 ]; then
+    echo "Both schemas are same"
+else
+    echo "Schema mismatch"
+    cat $DUMPDIR/diff.txt
+fi
 rm -rf $TMPDIR $DUMPDIR
 
 exit $diffErrorCode

services/mfa/mfa_test.go

@@ -41,9 +41,7 @@ func TestGenerateSecret(t *testing.T) {
 
 	assert.Len(t, secret, 32)
 
-	if len(img) == 0 {
+	require.NotEmpty(t, img, "no image set")
-		t.Fatal("no image set")
-	}
 
 	config.ServiceSettings.EnableMultifactorAuthentication = model.NewBool(false)
 

services/timezones/default.go

@@ -378,7 +378,6 @@ var DefaultSupportedTimezones = []string{
 	"CST6CDT",
 	"Canada/Atlantic",
 	"Canada/Central",
-	"Canada/East-Saskatchewan",
 	"Canada/Eastern",
 	"Canada/Mountain",
 	"Canada/Newfoundland",
@@ -491,7 +490,6 @@ var DefaultSupportedTimezones = []string{
 	"Europe/Zagreb",
 	"Europe/Zaporozhye",
 	"Europe/Zurich",
-	"Factory",
 	"GB",
 	"GB-Eire",
 	"GMT",

store/sqlstore/post_store.go

@@ -191,7 +191,7 @@ func (s *SqlPostStore) GetFlaggedPosts(userId string, offset int, limit int) (*m
 	pl := model.NewPostList()
 
 	var posts []*model.Post
-	if _, err := s.GetReplica().Select(&posts, "SELECT * FROM Posts WHERE Id IN (SELECT Name FROM Preferences WHERE UserId = :UserId AND Category = :Category) AND DeleteAt = 0 ORDER BY CreateAt DESC LIMIT :Limit OFFSET :Offset", map[string]interface{}{"UserId": userId, "Category": model.PREFERENCE_CATEGORY_FLAGGED_POST, "Offset": offset, "Limit": limit}); err != nil {
+	if _, err := s.GetReplica().Select(&posts, "SELECT *, (SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = p.Id AND Posts.DeleteAt = 0) as ReplyCount FROM Posts p WHERE Id IN (SELECT Name FROM Preferences WHERE UserId = :UserId AND Category = :Category) AND DeleteAt = 0 ORDER BY CreateAt DESC LIMIT :Limit OFFSET :Offset", map[string]interface{}{"UserId": userId, "Category": model.PREFERENCE_CATEGORY_FLAGGED_POST, "Offset": offset, "Limit": limit}); err != nil {
 		return nil, model.NewAppError("SqlPostStore.GetFlaggedPosts", "store.sql_post.get_flagged_posts.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
@@ -210,7 +210,7 @@ func (s *SqlPostStore) GetFlaggedPostsForTeam(userId, teamId string, offset int,
 
 	query := `
             SELECT
-                A.*
+                A.*, (SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = A.Id AND Posts.DeleteAt = 0) as ReplyCount 
             FROM
                 (SELECT
                     *
@@ -252,8 +252,8 @@ func (s *SqlPostStore) GetFlaggedPostsForChannel(userId, channelId string, offse
 	var posts []*model.Post
 	query := `
 		SELECT
-			*
+			*, (SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = p.Id AND Posts.DeleteAt = 0) as ReplyCount 
-		FROM Posts
+		FROM Posts p
 		WHERE
 			Id IN (SELECT Name FROM Preferences WHERE UserId = :UserId AND Category = :Category)
 			AND ChannelId = :ChannelId
@@ -280,12 +280,7 @@ func (s *SqlPostStore) Get(id string, skipFetchThreads bool) (*model.PostList, *
 	}
 
 	var post model.Post
-	var postFetchQuery string
+	postFetchQuery := "SELECT p.*, (SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = p.Id AND Posts.DeleteAt = 0) as ReplyCount FROM Posts p WHERE p.Id = :Id AND p.DeleteAt = 0"
-	if skipFetchThreads {
-		postFetchQuery = "SELECT p.*, (SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = p.Id) as ReplyCount  FROM Posts p WHERE p.Id = :Id AND p.DeleteAt = 0"
-	} else {
-		postFetchQuery = "SELECT * FROM Posts WHERE Id = :Id AND DeleteAt = 0"
-	}
 	err := s.GetReplica().SelectOne(&post, postFetchQuery, map[string]interface{}{"Id": id})
 	if err != nil {
 		return nil, model.NewAppError("SqlPostStore.GetPost", "store.sql_post.get.app_error", nil, "id="+id+err.Error(), http.StatusNotFound)
@@ -304,7 +299,7 @@ func (s *SqlPostStore) Get(id string, skipFetchThreads bool) (*model.PostList, *
 		}
 
 		var posts []*model.Post
-		_, err = s.GetReplica().Select(&posts, "SELECT * FROM Posts WHERE (Id = :Id OR RootId = :RootId) AND DeleteAt = 0", map[string]interface{}{"Id": rootId, "RootId": rootId})
+		_, err = s.GetReplica().Select(&posts, "SELECT *, (SELECT count(Id) FROM Posts WHERE RootId = p.Id AND Posts.DeleteAt = 0) as ReplyCount FROM Posts p WHERE (Id = :Id OR RootId = :RootId) AND DeleteAt = 0", map[string]interface{}{"Id": rootId, "RootId": rootId})
 		if err != nil {
 			return nil, model.NewAppError("SqlPostStore.GetPost", "store.sql_post.get.app_error", nil, "root_id="+rootId+err.Error(), http.StatusInternalServerError)
 		}
@@ -545,8 +540,8 @@ func (s *SqlPostStore) GetPostsSince(options model.GetPostsSinceOptions, allowFr
 	replyCountQuery1 := ""
 	replyCountQuery2 := ""
 	if options.SkipFetchThreads {
-		replyCountQuery1 = ` ,(SELECT COUNT(Posts.Id) FROM Posts WHERE p1.RootId = '' AND Posts.RootId = p1.Id) as ReplyCount`
+		replyCountQuery1 = ` ,(SELECT COUNT(Posts.Id) FROM Posts WHERE p1.RootId = '' AND Posts.RootId = p1.Id AND Posts.DeleteAt = 0) as ReplyCount`
-		replyCountQuery2 = ` ,(SELECT COUNT(Posts.Id) FROM Posts WHERE p2.RootId = '' AND Posts.RootId = p2.Id) as ReplyCount`
+		replyCountQuery2 = ` ,(SELECT COUNT(Posts.Id) FROM Posts WHERE p2.RootId = '' AND Posts.RootId = p2.Id AND Posts.DeleteAt = 0) as ReplyCount`
 	}
 
 	_, err := s.GetReplica().Select(&posts,
@@ -621,7 +616,7 @@ func (s *SqlPostStore) getPostsAround(before bool, options model.GetPostsOptions
 		direction = ">"
 		sort = "ASC"
 	}
-	replyCountSubQuery := s.getQueryBuilder().Select("COUNT(Posts.Id)").From("Posts").Where(sq.Expr("p.RootId = '' AND RootId = p.Id"))
+	replyCountSubQuery := s.getQueryBuilder().Select("COUNT(Posts.Id)").From("Posts").Where(sq.Expr("p.RootId = '' AND RootId = p.Id AND DeleteAt = 0"))
 	query := s.getQueryBuilder().Select("p.*")
 	if options.SkipFetchThreads {
 		query = query.Column(sq.Alias(replyCountSubQuery, "ReplyCount"))
@@ -782,7 +777,7 @@ func (s *SqlPostStore) getRootPosts(channelId string, offset int, limit int, ski
 	var posts []*model.Post
 	var fetchQuery string
 	if skipFetchThreads {
-		fetchQuery = "SELECT p.*, (SELECT COUNT(Posts.Id) FROM Posts WHERE p.RootId = '' AND Posts.RootId = p.Id) as ReplyCount FROM Posts p WHERE ChannelId = :ChannelId AND DeleteAt = 0 ORDER BY CreateAt DESC LIMIT :Limit OFFSET :Offset"
+		fetchQuery = "SELECT p.*, (SELECT COUNT(Posts.Id) FROM Posts WHERE p.RootId = '' AND Posts.RootId = p.Id AND Posts.DeleteAt = 0) as ReplyCount FROM Posts p WHERE ChannelId = :ChannelId AND DeleteAt = 0 ORDER BY CreateAt DESC LIMIT :Limit OFFSET :Offset"
 	} else {
 		fetchQuery = "SELECT * FROM Posts WHERE ChannelId = :ChannelId AND DeleteAt = 0 ORDER BY CreateAt DESC LIMIT :Limit OFFSET :Offset"
 	}
@@ -798,7 +793,7 @@ func (s *SqlPostStore) getParentsPosts(channelId string, offset int, limit int,
 	replyCountQuery := ""
 	onStatement := "q1.RootId = q2.Id"
 	if skipFetchThreads {
-		replyCountQuery = ` ,(SELECT COUNT(Posts.Id) FROM Posts WHERE q2.RootId = '' AND Posts.RootId = q2.Id) as ReplyCount`
+		replyCountQuery = ` ,(SELECT COUNT(Posts.Id) FROM Posts WHERE q2.RootId = '' AND Posts.RootId = q2.Id AND Posts.DeleteAt = 0) as ReplyCount`
 	} else {
 		onStatement += " OR q1.RootId = q2.RootId"
 	}
@@ -988,9 +983,9 @@ func (s *SqlPostStore) Search(teamId string, userId string, params *model.Search
 
 	searchQuery := `
 			SELECT
-				*
+				* ,(SELECT COUNT(Posts.Id) FROM Posts WHERE q2.RootId = '' AND Posts.RootId = q2.Id AND Posts.DeleteAt = 0) as ReplyCount
 			FROM
-				Posts
+				Posts q2
 			WHERE
 				DeleteAt = 0
 				AND Type NOT LIKE '` + model.POST_SYSTEM_MESSAGE_PREFIX + `%'

store/sqlstore/upgrade.go

@@ -713,17 +713,6 @@ func UpgradeDatabaseToVersion513(sqlStore SqlStore) {
 
 func UpgradeDatabaseToVersion514(sqlStore SqlStore) {
 	if shouldPerformUpgrade(sqlStore, VERSION_5_13_0, VERSION_5_14_0) {
-		sqlStore.AlterColumnTypeIfExists("TeamMembers", "SchemeGuest", "tinyint(4)", "boolean")
-		sqlStore.AlterColumnTypeIfExists("ChannelMembers", "SchemeGuest", "tinyint(4)", "boolean")
-		sqlStore.AlterColumnTypeIfExists("Schemes", "DefaultTeamGuestRole", "varchar(64)", "VARCHAR(64)")
-		sqlStore.AlterColumnTypeIfExists("Schemes", "DefaultChannelGuestRole", "varchar(64)", "VARCHAR(64)")
-		sqlStore.AlterColumnTypeIfExists("Teams", "AllowedDomains", "text", "VARCHAR(1000)")
-		sqlStore.AlterColumnTypeIfExists("Channels", "GroupConstrained", "tinyint(1)", "boolean")
-		sqlStore.AlterColumnTypeIfExists("Teams", "GroupConstrained", "tinyint(1)", "boolean")
-
-		sqlStore.CreateIndexIfNotExists("idx_groupteams_teamid", "GroupTeams", "TeamId")
-		sqlStore.CreateIndexIfNotExists("idx_groupchannels_channelid", "GroupChannels", "ChannelId")
-
 		saveSchemaVersion(sqlStore, VERSION_5_14_0)
 	}
 }
@@ -742,5 +731,21 @@ func UpgradeDatabaseToVersion516(sqlStore SqlStore) {
 			sqlStore.GetMaster().Exec("ALTER TABLE Tokens MODIFY Extra text")
 		}
 		saveSchemaVersion(sqlStore, VERSION_5_16_0)
+
+		// Fix mismatches between the canonical and migrated schemas.
+		sqlStore.AlterColumnTypeIfExists("TeamMembers", "SchemeGuest", "tinyint(4)", "boolean")
+		sqlStore.AlterColumnTypeIfExists("Schemes", "DefaultTeamGuestRole", "varchar(64)", "VARCHAR(64)")
+		sqlStore.AlterColumnTypeIfExists("Schemes", "DefaultChannelGuestRole", "varchar(64)", "VARCHAR(64)")
+		sqlStore.AlterColumnTypeIfExists("Teams", "AllowedDomains", "text", "VARCHAR(1000)")
+		sqlStore.AlterColumnTypeIfExists("Channels", "GroupConstrained", "tinyint(1)", "boolean")
+		sqlStore.AlterColumnTypeIfExists("Teams", "GroupConstrained", "tinyint(1)", "boolean")
+
+		// One known mismatch remains: ChannelMembers.SchemeGuest. The requisite migration
+		// is left here for posterity, but we're avoiding fix this given the corresponding
+		// table rewrite in most MySQL and Postgres instances.
+		// sqlStore.AlterColumnTypeIfExists("ChannelMembers", "SchemeGuest", "tinyint(4)", "boolean")
+
+		sqlStore.CreateIndexIfNotExists("idx_groupteams_teamid", "GroupTeams", "TeamId")
+		sqlStore.CreateIndexIfNotExists("idx_groupchannels_channelid", "GroupChannels", "ChannelId")
 	}
 }

store/storetest/group_store.go

@@ -2018,7 +2018,7 @@ func testGetGroups(t *testing.T, ss store.Store) {
 	require.Nil(t, err)
 
 	group2, err := ss.Group().Create(&model.Group{
-		Name:        model.NewId(),
+		Name:        model.NewId() + "-group-2",
 		DisplayName: "group-2",
 		RemoteId:    model.NewId(),
 		Source:      model.GroupSourceLdap,
@@ -2062,7 +2062,7 @@ func testGetGroups(t *testing.T, ss store.Store) {
 
 	// Create Group3
 	group3, err := ss.Group().Create(&model.Group{
-		Name:        model.NewId(),
+		Name:        model.NewId() + "-group-3",
 		DisplayName: "group-3",
 		RemoteId:    model.NewId(),
 		Source:      model.GroupSourceLdap,
@@ -2122,7 +2122,7 @@ func testGetGroups(t *testing.T, ss store.Store) {
 	user2.DeleteAt = 1
 	ss.User().Update(user2, true)
 
-	group2NameSubstring := string([]rune(group2.Name)[2:5])
+	group2NameSubstring := "group-2"
 
 	testCases := []struct {
 		Name    string

store/storetest/session_store.go

@@ -34,9 +34,8 @@ func testSessionStoreSave(t *testing.T, ss store.Store) {
 	s1 := &model.Session{}
 	s1.UserId = model.NewId()
 
-	if _, err := ss.Session().Save(s1); err != nil {
+	_, err := ss.Session().Save(s1)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 }
 
 func testSessionGet(t *testing.T, ss store.Store) {
@@ -59,21 +58,13 @@ func testSessionGet(t *testing.T, ss store.Store) {
 	s3, err = ss.Session().Save(s3)
 	require.Nil(t, err)
 
-	if session, err := ss.Session().Get(s1.Id); err != nil {
+	session, err := ss.Session().Get(s1.Id)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else {
+	require.Equal(t, session.Id, s1.Id, "should match")
-		if session.Id != s1.Id {
-			t.Fatal("should match")
-		}
-	}
 
-	if session, err := ss.Session().GetSessions(s1.UserId); err != nil {
+	data, err := ss.Session().GetSessions(s1.UserId)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else {
+	require.Len(t, data, 3, "should match len")
-		if len(session) != 3 {
-			t.Fatal("should match len")
-		}
-	}
 }
 
 func testSessionGetWithDeviceId(t *testing.T, ss store.Store) {
@@ -100,13 +91,9 @@ func testSessionGetWithDeviceId(t *testing.T, ss store.Store) {
 	s3, err = ss.Session().Save(s3)
 	require.Nil(t, err)
 
-	if data, err := ss.Session().GetSessionsWithActiveDeviceIds(s1.UserId); err != nil {
+	data, err := ss.Session().GetSessionsWithActiveDeviceIds(s1.UserId)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else {
+	require.Len(t, data, 1, "should match len")
-		if len(data) != 1 {
-			t.Fatal("should match len")
-		}
-	}
 }
 
 func testSessionRemove(t *testing.T, ss store.Store) {
@@ -116,19 +103,15 @@ func testSessionRemove(t *testing.T, ss store.Store) {
 	s1, err := ss.Session().Save(s1)
 	require.Nil(t, err)
 
-	if session, err := ss.Session().Get(s1.Id); err != nil {
+	session, err := ss.Session().Get(s1.Id)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else {
+	require.Equal(t, session.Id, s1.Id, "should match")
-		if session.Id != s1.Id {
-			t.Fatal("should match")
-		}
-	}
 
 	removeErr := ss.Session().Remove(s1.Id)
 	require.Nil(t, removeErr)
-	if _, err := ss.Session().Get(s1.Id); err == nil {
+
-		t.Fatal("should have been removed")
+	_, err = ss.Session().Get(s1.Id)
-	}
+	require.NotNil(t, err, "should have been removed")
 }
 
 func testSessionRemoveAll(t *testing.T, ss store.Store) {
@@ -138,20 +121,15 @@ func testSessionRemoveAll(t *testing.T, ss store.Store) {
 	s1, err := ss.Session().Save(s1)
 	require.Nil(t, err)
 
-	if session, err := ss.Session().Get(s1.Id); err != nil {
+	session, err := ss.Session().Get(s1.Id)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else {
+	require.Equal(t, session.Id, s1.Id, "should match")
-		if session.Id != s1.Id {
-			t.Fatal("should match")
-		}
-	}
 
 	removeErr := ss.Session().RemoveAllSessions()
 	require.Nil(t, removeErr)
 
-	if _, err := ss.Session().Get(s1.Id); err == nil {
+	_, err = ss.Session().Get(s1.Id)
-		t.Fatal("should have been removed")
+	require.NotNil(t, err, "should have been removed")
-	}
 }
 
 func testSessionRemoveByUser(t *testing.T, ss store.Store) {
@@ -161,20 +139,15 @@ func testSessionRemoveByUser(t *testing.T, ss store.Store) {
 	s1, err := ss.Session().Save(s1)
 	require.Nil(t, err)
 
-	if session, err := ss.Session().Get(s1.Id); err != nil {
+	session, err := ss.Session().Get(s1.Id)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else {
+	require.Equal(t, session.Id, s1.Id, "should match")
-		if session.Id != s1.Id {
-			t.Fatal("should match")
-		}
-	}
 
 	deleteErr := ss.Session().PermanentDeleteSessionsByUser(s1.UserId)
 	require.Nil(t, deleteErr)
 
-	if _, err := ss.Session().Get(s1.Id); err == nil {
+	_, err = ss.Session().Get(s1.Id)
-		t.Fatal("should have been removed")
+	require.NotNil(t, err, "should have been removed")
-	}
 }
 
 func testSessionRemoveToken(t *testing.T, ss store.Store) {
@@ -184,28 +157,19 @@ func testSessionRemoveToken(t *testing.T, ss store.Store) {
 	s1, err := ss.Session().Save(s1)
 	require.Nil(t, err)
 
-	if session, err := ss.Session().Get(s1.Id); err != nil {
+	session, err := ss.Session().Get(s1.Id)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else {
+	require.Equal(t, session.Id, s1.Id, "should match")
-		if session.Id != s1.Id {
-			t.Fatal("should match")
-		}
-	}
 
 	removeErr := ss.Session().Remove(s1.Token)
 	require.Nil(t, removeErr)
 
-	if _, err := ss.Session().Get(s1.Id); err == nil {
+	_, err = ss.Session().Get(s1.Id)
-		t.Fatal("should have been removed")
+	require.NotNil(t, err, "should have been removed")
-	}
 
-	if session, err := ss.Session().GetSessions(s1.UserId); err != nil {
+	data, err := ss.Session().GetSessions(s1.UserId)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else {
+	require.Len(t, data, 0, "should match len")
-		if len(session) != 0 {
-			t.Fatal("should match len")
-		}
-	}
 }
 
 func testSessionUpdateDeviceId(t *testing.T, ss store.Store) {
@@ -215,9 +179,8 @@ func testSessionUpdateDeviceId(t *testing.T, ss store.Store) {
 	s1, err := ss.Session().Save(s1)
 	require.Nil(t, err)
 
-	if _, err = ss.Session().UpdateDeviceId(s1.Id, model.PUSH_NOTIFY_APPLE+":1234567890", s1.ExpiresAt); err != nil {
+	_, err = ss.Session().UpdateDeviceId(s1.Id, model.PUSH_NOTIFY_APPLE+":1234567890", s1.ExpiresAt)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
 	s2 := &model.Session{}
 	s2.UserId = model.NewId()
@@ -225,9 +188,8 @@ func testSessionUpdateDeviceId(t *testing.T, ss store.Store) {
 	s2, err = ss.Session().Save(s2)
 	require.Nil(t, err)
 
-	if _, err := ss.Session().UpdateDeviceId(s2.Id, model.PUSH_NOTIFY_APPLE+":1234567890", s1.ExpiresAt); err != nil {
+	_, err = ss.Session().UpdateDeviceId(s2.Id, model.PUSH_NOTIFY_APPLE+":1234567890", s1.ExpiresAt)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 }
 
 func testSessionUpdateDeviceId2(t *testing.T, ss store.Store) {
@@ -237,9 +199,8 @@ func testSessionUpdateDeviceId2(t *testing.T, ss store.Store) {
 	s1, err := ss.Session().Save(s1)
 	require.Nil(t, err)
 
-	if _, err = ss.Session().UpdateDeviceId(s1.Id, model.PUSH_NOTIFY_APPLE_REACT_NATIVE+":1234567890", s1.ExpiresAt); err != nil {
+	_, err = ss.Session().UpdateDeviceId(s1.Id, model.PUSH_NOTIFY_APPLE_REACT_NATIVE+":1234567890", s1.ExpiresAt)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
 	s2 := &model.Session{}
 	s2.UserId = model.NewId()
@@ -247,9 +208,8 @@ func testSessionUpdateDeviceId2(t *testing.T, ss store.Store) {
 	s2, err = ss.Session().Save(s2)
 	require.Nil(t, err)
 
-	if _, err := ss.Session().UpdateDeviceId(s2.Id, model.PUSH_NOTIFY_APPLE_REACT_NATIVE+":1234567890", s1.ExpiresAt); err != nil {
+	_, err = ss.Session().UpdateDeviceId(s2.Id, model.PUSH_NOTIFY_APPLE_REACT_NATIVE+":1234567890", s1.ExpiresAt)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 }
 
 func testSessionStoreUpdateLastActivityAt(t *testing.T, ss store.Store) {
@@ -262,14 +222,9 @@ func testSessionStoreUpdateLastActivityAt(t *testing.T, ss store.Store) {
 	err = ss.Session().UpdateLastActivityAt(s1.Id, 1234567890)
 	require.Nil(t, err)
 
-	if session, err := ss.Session().Get(s1.Id); err != nil {
+	session, err := ss.Session().Get(s1.Id)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else {
+	require.EqualValues(t, session.LastActivityAt, 1234567890, "LastActivityAt not updated correctly")
-		if session.LastActivityAt != 1234567890 {
-			t.Fatal("LastActivityAt not updated correctly")
-		}
-	}
-
 }
 
 func testSessionCount(t *testing.T, ss store.Store) {
@@ -280,13 +235,9 @@ func testSessionCount(t *testing.T, ss store.Store) {
 	s1, err := ss.Session().Save(s1)
 	require.Nil(t, err)
 
-	if count, err := ss.Session().AnalyticsSessionCount(); err != nil {
+	count, err := ss.Session().AnalyticsSessionCount()
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else {
+	require.NotZero(t, count, "should have at least 1 session")
-		if count == 0 {
-			t.Fatal("should have at least 1 session")
-		}
-	}
 }
 
 func testSessionCleanup(t *testing.T, ss store.Store) {

utils/subpath.go

@@ -148,6 +148,12 @@ func UpdateAssetsSubpathFromConfig(config *model.Config) error {
 		return nil
 	}
 
+	// Similarly, don't rewrite during a CI build, when the assets may not even be present.
+	if os.Getenv("IS_CI") == "true" {
+		mlog.Debug("Skipping update to assets subpath since CI build")
+		return nil
+	}
+
 	subpath, err := GetSubpathFromConfig(config)
 	if err != nil {
 		return err

utils/subpath_test.go

@@ -14,6 +14,40 @@ import (
 	"github.com/mattermost/mattermost-server/utils"
 )
 
+func TestUpdateAssetsSubpathFromConfig(t *testing.T) {
+	t.Run("dev build", func(t *testing.T) {
+		var oldBuildNumber = model.BuildNumber
+		model.BuildNumber = "dev"
+		defer func() {
+			model.BuildNumber = oldBuildNumber
+		}()
+
+		err := utils.UpdateAssetsSubpathFromConfig(nil)
+		require.NoError(t, err)
+	})
+
+	t.Run("IS_CI=true", func(t *testing.T) {
+		err := os.Setenv("IS_CI", "true")
+		require.NoError(t, err)
+		defer func() {
+			os.Unsetenv("IS_CI")
+		}()
+
+		err = utils.UpdateAssetsSubpathFromConfig(nil)
+		require.NoError(t, err)
+	})
+
+	t.Run("no config", func(t *testing.T) {
+		tempDir, err := ioutil.TempDir("", "test_update_assets_subpath")
+		require.NoError(t, err)
+		defer os.RemoveAll(tempDir)
+		os.Chdir(tempDir)
+
+		err = utils.UpdateAssetsSubpathFromConfig(nil)
+		require.Error(t, err)
+	})
+}
+
 func TestUpdateAssetsSubpath(t *testing.T) {
 	t.Run("no client dir", func(t *testing.T) {
 		tempDir, err := ioutil.TempDir("", "test_update_assets_subpath")
@@ -157,10 +191,6 @@ func TestUpdateAssetsSubpath(t *testing.T) {
 }
 
 func TestGetSubpathFromConfig(t *testing.T) {
-	sToP := func(s string) *string {
-		return &s
-	}
-
 	testCases := []struct {
 		Description     string
 		SiteURL         *string
@@ -231,6 +261,10 @@ func TestGetSubpathFromConfig(t *testing.T) {
 	}
 }
 
+func sToP(s string) *string {
+	return &s
+}
+
 const contentSecurityPolicyNotFoundHtml = `<!DOCTYPE html> <html lang=en> <head> <meta charset=utf-8> <meta http-equiv=Content-Security-Policy content="script-src 'self' cdn.segment.com/analytics.js/"> <meta http-equiv=X-UA-Compatible content="IE=edge"> <meta name=viewport content="width=device-width,initial-scale=1,maximum-scale=1,user-scalable=0"> <meta name=robots content="noindex, nofollow"> <meta name=referrer content=no-referrer> <title>Mattermost</title> <meta name=apple-mobile-web-app-capable content=yes> <meta name=apple-mobile-web-app-status-bar-style content=default> <meta name=mobile-web-app-capable content=yes> <meta name=apple-mobile-web-app-title content=Mattermost> <meta name=application-name content=Mattermost> <meta name=format-detection content="telephone=no"> <link rel=apple-touch-icon sizes=57x57 href=/static/files/78b7e73b41b8731ce2c41c870ecc8886.png> <link rel=apple-touch-icon sizes=60x60 href=/static/files/51d00ffd13afb6d74fd8f6dfdeef768a.png> <link rel=apple-touch-icon sizes=72x72 href=/static/files/23645596f8f78f017bd4d457abb855c4.png> <link rel=apple-touch-icon sizes=76x76 href=/static/files/26e9d72f472663a00b4b206149459fab.png> <link rel=apple-touch-icon sizes=144x144 href=/static/files/7bd91659bf3fc8c68fcd45fc1db9c630.png> <link rel=apple-touch-icon sizes=120x120 href=/static/files/fa69ffe11eb334aaef5aece8d848ca62.png> <link rel=apple-touch-icon sizes=152x152 href=/static/files/f046777feb6ab12fc43b8f9908b1db35.png> <link rel=icon type=image/png sizes=16x16 href=/static/files/02b96247d275680adaaabf01c71c571d.png> <link rel=icon type=image/png sizes=32x32 href=/static/files/1d9020f201a6762421cab8d30624fdd8.png> <link rel=icon type=image/png sizes=96x96 href=/static/files/fe23af39ae98d77dc26ae8586565970f.png> <link rel=icon type=image/png sizes=192x192 href=/static/files/d7ff68a7675f84337cc154c3d4abe713.png> <link rel=manifest href=/static/files/a985ad72552ad069537d6eea81e719c7.json> <link rel=stylesheet class=code_theme> <style>.error-screen{font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding-top:50px;max-width:750px;font-size:14px;color:#333;margin:auto;display:none;line-height:1.5}.error-screen h2{font-size:30px;font-weight:400;line-height:1.2}.error-screen ul{padding-left:15px;line-height:1.7;margin-top:0;margin-bottom:10px}.error-screen hr{color:#ddd;margin-top:20px;margin-bottom:20px;border:0;border-top:1px solid #eee}.error-screen-visible{display:block}</style> <link href="/static/main.364fd054d7a6d741efc6.css" rel="stylesheet"><script type="text/javascript" src="/static/main.e49599ac425584ffead5.js"></script></head> <body class=font--open_sans> <div id=root> <div class=error-screen> <h2>Cannot connect to Mattermost</h2> <hr/> <p>We're having trouble connecting to Mattermost. If refreshing this page (Ctrl+R or Command+R) does not work, please verify that your computer is connected to the internet.</p> <br/> </div> <div class=loading-screen style=position:relative> <div class=loading__content> <div class="round round-1"></div> <div class="round round-2"></div> <div class="round round-3"></div> </div> </div> </div> <noscript> To use Mattermost, please enable JavaScript. </noscript> </body> </html>`
 
 const contentSecurityPolicyNotFound2Html = `<!DOCTYPE html> <html lang=en> <head> <meta charset=utf-8> <meta http-equiv=Content-Security-Policy content="script-src 'self' cdn.segment.com/analytics.js/ 'unsafe-eval'"> <meta http-equiv=X-UA-Compatible content="IE=edge"> <meta name=viewport content="width=device-width,initial-scale=1,maximum-scale=1,user-scalable=0"> <meta name=robots content="noindex, nofollow"> <meta name=referrer content=no-referrer> <title>Mattermost</title> <meta name=apple-mobile-web-app-capable content=yes> <meta name=apple-mobile-web-app-status-bar-style content=default> <meta name=mobile-web-app-capable content=yes> <meta name=apple-mobile-web-app-title content=Mattermost> <meta name=application-name content=Mattermost> <meta name=format-detection content="telephone=no"> <link rel=apple-touch-icon sizes=57x57 href=/static/files/78b7e73b41b8731ce2c41c870ecc8886.png> <link rel=apple-touch-icon sizes=60x60 href=/static/files/51d00ffd13afb6d74fd8f6dfdeef768a.png> <link rel=apple-touch-icon sizes=72x72 href=/static/files/23645596f8f78f017bd4d457abb855c4.png> <link rel=apple-touch-icon sizes=76x76 href=/static/files/26e9d72f472663a00b4b206149459fab.png> <link rel=apple-touch-icon sizes=144x144 href=/static/files/7bd91659bf3fc8c68fcd45fc1db9c630.png> <link rel=apple-touch-icon sizes=120x120 href=/static/files/fa69ffe11eb334aaef5aece8d848ca62.png> <link rel=apple-touch-icon sizes=152x152 href=/static/files/f046777feb6ab12fc43b8f9908b1db35.png> <link rel=icon type=image/png sizes=16x16 href=/static/files/02b96247d275680adaaabf01c71c571d.png> <link rel=icon type=image/png sizes=32x32 href=/static/files/1d9020f201a6762421cab8d30624fdd8.png> <link rel=icon type=image/png sizes=96x96 href=/static/files/fe23af39ae98d77dc26ae8586565970f.png> <link rel=icon type=image/png sizes=192x192 href=/static/files/d7ff68a7675f84337cc154c3d4abe713.png> <link rel=manifest href=/static/files/a985ad72552ad069537d6eea81e719c7.json> <link rel=stylesheet class=code_theme> <style>.error-screen{font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding-top:50px;max-width:750px;font-size:14px;color:#333;margin:auto;display:none;line-height:1.5}.error-screen h2{font-size:30px;font-weight:400;line-height:1.2}.error-screen ul{padding-left:15px;line-height:1.7;margin-top:0;margin-bottom:10px}.error-screen hr{color:#ddd;margin-top:20px;margin-bottom:20px;border:0;border-top:1px solid #eee}.error-screen-visible{display:block}</style> <link href="/static/main.364fd054d7a6d741efc6.css" rel="stylesheet"><script type="text/javascript" src="/static/main.e49599ac425584ffead5.js"></script></head> <body class=font--open_sans> <div id=root> <div class=error-screen> <h2>Cannot connect to Mattermost</h2> <hr/> <p>We're having trouble connecting to Mattermost. If refreshing this page (Ctrl+R or Command+R) does not work, please verify that your computer is connected to the internet.</p> <br/> </div> <div class=loading-screen style=position:relative> <div class=loading__content> <div class="round round-1"></div> <div class="round round-2"></div> <div class="round round-3"></div> </div> </div> </div> <noscript> To use Mattermost, please enable JavaScript. </noscript> </body> </html>`