MM-27007 Remove automatic sidebar migration (#15087)

* MM-27007 Add migration of favorited channels to CreateInitialSidebarCategories

* MM-27007 Rewrite migrateFavortitesToSidebarT to use ROW_NUMBER() when available

* MM-27007 Remove automatic sidebar migration

* Remove old i18n strings

* Fix typo

* Address feedback

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>

i18n/en.json

@@ -5238,14 +5238,6 @@
     "id": "migrations.worker.run_migration.unknown_key",
     "translation": "Unable to run migration job due to unknown migration key."
   },
-  {
-    "id": "migrations.worker.run_sidebar_categories_phase_2_migration.internal_error",
-    "translation": "Migration failed due to database error."
-  },
-  {
-    "id": "migrations.worker.run_sidebar_categories_phase_2_migration.invalid_progress",
-    "translation": "Migration failed due to invalid progress data."
-  },
   {
     "id": "model.access.is_valid.access_token.app_error",
     "translation": "Invalid access token."

migrations/migrations.go

@@ -32,7 +32,6 @@ func init() {
 func MakeMigrationsList() []string {
 	return []string{
 		model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2,
-		model.MIGRATION_KEY_SIDEBAR_CATEGORIES_PHASE_2,
 	}
 }
 

migrations/sidebar_categories_phase_2.go

@@ -1,128 +0,0 @@
-// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
-// See LICENSE.txt for license information.
-
-package migrations
-
-import (
-	"encoding/json"
-	"io"
-	"net/http"
-	"strings"
-
-	"github.com/mattermost/mattermost-server/v5/model"
-)
-
-type ProgressStep string
-
-const (
-	StepCategories ProgressStep = "populateSidebarCategories"
-	StepFavorites  ProgressStep = "migrateFavoriteChannelToSidebarChannels"
-	StepEnd        ProgressStep = "endMigration"
-)
-
-type Progress struct {
-	CurrentStep   ProgressStep `json:"current_state"`
-	LastTeamId    string       `json:"last_team_id"`
-	LastChannelId string       `json:"last_channel_id"`
-	LastUserId    string       `json:"last_user"`
-	LastSortOrder int64        `json:"last_sort_order"`
-}
-
-func (p *Progress) ToJson() string {
-	b, _ := json.Marshal(p)
-	return string(b)
-}
-
-func ProgressFromJson(data io.Reader) *Progress {
-	var o *Progress
-	json.NewDecoder(data).Decode(&o)
-	return o
-}
-
-func (p *Progress) IsValid() bool {
-	if len(p.LastChannelId) != 26 {
-		return false
-	}
-
-	if len(p.LastTeamId) != 26 {
-		return false
-	}
-
-	if len(p.LastUserId) != 26 {
-		return false
-	}
-
-	switch p.CurrentStep {
-	case StepCategories, StepFavorites:
-		return true
-	default:
-		return false
-	}
-}
-
-func newProgress(step ProgressStep) *Progress {
-	progress := new(Progress)
-	progress.CurrentStep = step
-	progress.LastChannelId = strings.Repeat("0", 26)
-	progress.LastTeamId = strings.Repeat("0", 26)
-	progress.LastUserId = strings.Repeat("0", 26)
-	progress.LastSortOrder = 0
-	return progress
-}
-
-func (worker *Worker) runSidebarCategoriesPhase2Migration(lastDone string) (bool, string, *model.AppError) {
-	var progress *Progress
-	if len(lastDone) == 0 {
-		progress = newProgress(StepCategories)
-	} else {
-		progress = ProgressFromJson(strings.NewReader(lastDone))
-		if !progress.IsValid() {
-			return false, "", model.NewAppError("MigrationsWorker.runSidebarCategoriesPhase2Migration", "migrations.worker.run_sidebar_categories_phase_2_migration.invalid_progress", map[string]interface{}{"progress": progress.ToJson()}, "", http.StatusInternalServerError)
-		}
-	}
-
-	var result map[string]interface{}
-	var nErr error
-	var nextStep ProgressStep
-	switch progress.CurrentStep {
-	case StepCategories:
-		result, nErr = worker.srv.Store.Channel().MigrateSidebarCategories(progress.LastTeamId, progress.LastUserId)
-		nextStep = StepFavorites
-	case StepFavorites:
-		result, nErr = worker.srv.Store.Channel().MigrateFavoritesToSidebarChannels(progress.LastUserId, progress.LastSortOrder)
-		nextStep = StepEnd
-	default:
-		return false, "", model.NewAppError("MigrationsWorker.runSidebarCategoriesPhase2Migration", "migrations.worker.run_sidebar_categories_phase_2_migration.invalid_progress", map[string]interface{}{"progress": progress.ToJson()}, "", http.StatusInternalServerError)
-	}
-
-	if nErr != nil {
-		return false, progress.ToJson(), model.NewAppError("MigrationsWorker.runSidebarCategoriesPhase2Migration", "migrations.worker.run_sidebar_categories_phase_2_migration.internal_error", nil, nErr.Error(), http.StatusInternalServerError)
-	}
-
-	if result == nil {
-		// We haven't progressed. That means that we've reached the end of this stage of the migration, and should now advance to the next stage or stop
-		if nextStep != StepEnd {
-			progress = newProgress(nextStep)
-			return false, progress.ToJson(), nil
-		}
-		return true, progress.ToJson(), nil
-	}
-
-	progress.LastChannelId = strings.Repeat("0", 26)
-	progress.LastTeamId = strings.Repeat("0", 26)
-	progress.LastUserId = strings.Repeat("0", 26)
-	progress.LastSortOrder = 0
-	if val, ok := result["UserId"].(string); ok {
-		progress.LastUserId = val
-	}
-	if val, ok := result["TeamId"].(string); ok {
-		progress.LastTeamId = val
-	}
-	if val, ok := result["ChannelId"].(string); ok {
-		progress.LastChannelId = val
-	}
-	if val, ok := result["SortOrder"].(int64); ok {
-		progress.LastSortOrder = val
-	}
-	return false, progress.ToJson(), nil
-}

migrations/worker.go

@@ -150,8 +150,6 @@ func (worker *Worker) runMigration(key string, lastDone string) (bool, string, *
 	var err *model.AppError
 
 	switch key {
-	case model.MIGRATION_KEY_SIDEBAR_CATEGORIES_PHASE_2:
-		done, progress, err = worker.runSidebarCategoriesPhase2Migration(lastDone)
 	case model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2:
 		done, progress, err = worker.runAdvancedPermissionsPhase2Migration(lastDone)
 	default:

model/migration.go

@@ -17,6 +17,4 @@ const (
 	MIGRATION_KEY_ADD_MANAGE_GUESTS_PERMISSIONS               = "add_manage_guests_permissions"
 	MIGRATION_KEY_CHANNEL_MODERATIONS_PERMISSIONS             = "channel_moderations_permissions"
 	MIGRATION_KEY_ADD_USE_GROUP_MENTIONS_PERMISSION           = "add_use_group_mentions_permission"
-
-	MIGRATION_KEY_SIDEBAR_CATEGORIES_PHASE_2 = "migration_sidebar_categories_phase_2"
 )

store/opentracing_layer.go

@@ -1647,24 +1647,6 @@ func (s *OpenTracingLayerChannelStore) MigrateChannelMembers(fromChannelId strin
 	return resultVar0, resultVar1
 }
 
-func (s *OpenTracingLayerChannelStore) MigrateFavoritesToSidebarChannels(lastUserId string, runningOrder int64) (map[string]interface{}, error) {
-	origCtx := s.Root.Store.Context()
-	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ChannelStore.MigrateFavoritesToSidebarChannels")
-	s.Root.Store.SetContext(newCtx)
-	defer func() {
-		s.Root.Store.SetContext(origCtx)
-	}()
-
-	defer span.Finish()
-	resultVar0, resultVar1 := s.ChannelStore.MigrateFavoritesToSidebarChannels(lastUserId, runningOrder)
-	if resultVar1 != nil {
-		span.LogFields(spanlog.Error(resultVar1))
-		ext.Error.Set(span, true)
-	}
-
-	return resultVar0, resultVar1
-}
-
 func (s *OpenTracingLayerChannelStore) MigratePublicChannels() error {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ChannelStore.MigratePublicChannels")
@@ -1683,24 +1665,6 @@ func (s *OpenTracingLayerChannelStore) MigratePublicChannels() error {
 	return resultVar0
 }
 
-func (s *OpenTracingLayerChannelStore) MigrateSidebarCategories(fromTeamId string, fromUserId string) (map[string]interface{}, error) {
-	origCtx := s.Root.Store.Context()
-	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ChannelStore.MigrateSidebarCategories")
-	s.Root.Store.SetContext(newCtx)
-	defer func() {
-		s.Root.Store.SetContext(origCtx)
-	}()
-
-	defer span.Finish()
-	resultVar0, resultVar1 := s.ChannelStore.MigrateSidebarCategories(fromTeamId, fromUserId)
-	if resultVar1 != nil {
-		span.LogFields(spanlog.Error(resultVar1))
-		ext.Error.Set(span, true)
-	}
-
-	return resultVar0, resultVar1
-}
-
 func (s *OpenTracingLayerChannelStore) PermanentDelete(channelId string) error {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ChannelStore.PermanentDelete")

store/sqlstore/channel_store.go

@@ -436,46 +436,6 @@ func (s SqlChannelStore) createIndexesIfNotExists() {
 	s.CreateIndexIfNotExists("idx_channels_scheme_id", "Channels", "SchemeId")
 }
 
-// MigrateSidebarCategories creates 3 initial categories for all existing user/team pairs
-// **IMPORTANT** This function should only be called from the migration task and shouldn't be used by itself
-func (s SqlChannelStore) MigrateSidebarCategories(fromTeamId, fromUserId string) (map[string]interface{}, error) {
-	var userTeamMap []struct {
-		UserId string
-		TeamId string
-	}
-
-	transaction, err := s.GetMaster().Begin()
-	if err != nil {
-		return nil, err
-	}
-
-	defer finalizeTransaction(transaction)
-
-	if _, err := transaction.Select(&userTeamMap, "SELECT TeamId, UserId FROM TeamMembers LEFT JOIN Users ON Users.Id=UserId WHERE (TeamId, UserId) > (:FromTeamId, :FromUserId) ORDER BY TeamId, UserId LIMIT 100", map[string]interface{}{"FromTeamId": fromTeamId, "FromUserId": fromUserId}); err != nil {
-		return nil, err
-	}
-
-	if len(userTeamMap) == 0 {
-		// No more team members in query result means that the migration has finished.
-		return nil, nil
-	}
-
-	for _, u := range userTeamMap {
-		if err := s.createInitialSidebarCategoriesT(transaction, u.UserId, u.TeamId); err != nil {
-			return nil, err
-		}
-	}
-	if err := transaction.Commit(); err != nil {
-		return nil, err
-	}
-
-	data := make(map[string]interface{})
-	data["TeamId"] = userTeamMap[len(userTeamMap)-1].TeamId
-	data["UserId"] = userTeamMap[len(userTeamMap)-1].UserId
-
-	return data, nil
-}
-
 func (s SqlChannelStore) CreateInitialSidebarCategories(userId, teamId string) error {
 	transaction, err := s.GetMaster().Begin()
 	if err != nil {
@@ -510,20 +470,22 @@ func (s SqlChannelStore) createInitialSidebarCategoriesT(transaction *gorp.Trans
 		return errors.Wrap(err, "createInitialSidebarCategoriesT: failed to select existing categories")
 	}
 
-	hasCategoryOfType := func(categoryType model.SidebarCategoryType) bool {
+	hasCategoryOfType := make(map[model.SidebarCategoryType]bool, len(existingTypes))
-		for _, existingType := range existingTypes {
+	for _, existingType := range existingTypes {
-			if categoryType == existingType {
+		hasCategoryOfType[existingType] = true
-				return true
-			}
-		}
-
-		return false
 	}
 
-	if !hasCategoryOfType(model.SidebarCategoryFavorites) {
+	if !hasCategoryOfType[model.SidebarCategoryFavorites] {
+		favoritesCategoryId := model.NewId()
+
+		// Create the SidebarChannels first since there's more opportunity for something to fail here
+		if err := s.migrateFavoritesToSidebarT(transaction, userId, teamId, favoritesCategoryId); err != nil {
+			return errors.Wrap(err, "createInitialSidebarCategoriesT: failed to migrate favorites to sidebar")
+		}
+
 		if err := transaction.Insert(&model.SidebarCategory{
 			DisplayName: "Favorites", // This will be retranslated by the client into the user's locale
-			Id:          model.NewId(),
+			Id:          favoritesCategoryId,
 			UserId:      userId,
 			TeamId:      teamId,
 			Sorting:     model.SidebarCategorySortDefault,
@@ -534,7 +496,7 @@ func (s SqlChannelStore) createInitialSidebarCategoriesT(transaction *gorp.Trans
 		}
 	}
 
-	if !hasCategoryOfType(model.SidebarCategoryChannels) {
+	if !hasCategoryOfType[model.SidebarCategoryChannels] {
 		if err := transaction.Insert(&model.SidebarCategory{
 			DisplayName: "Channels", // This will be retranslateed by the client into the user's locale
 			Id:          model.NewId(),
@@ -548,7 +510,7 @@ func (s SqlChannelStore) createInitialSidebarCategoriesT(transaction *gorp.Trans
 		}
 	}
 
-	if !hasCategoryOfType(model.SidebarCategoryDirectMessages) {
+	if !hasCategoryOfType[model.SidebarCategoryDirectMessages] {
 		if err := transaction.Insert(&model.SidebarCategory{
 			DisplayName: "Direct Messages", // This will be retranslateed by the client into the user's locale
 			Id:          model.NewId(),
@@ -595,6 +557,45 @@ func (s SqlChannelStore) migrateMembershipToSidebar(transaction *gorp.Transactio
 	return memberships, nil
 }
 
+func (s SqlChannelStore) migrateFavoritesToSidebarT(transaction *gorp.Transaction, userId, teamId, favoritesCategoryId string) error {
+	favoritesQuery, favoritesParams, _ := s.getQueryBuilder().
+		Select("Preferences.Name").
+		From("Preferences").
+		Join("Channels on Preferences.Name = Channels.Id").
+		Join("ChannelMembers on Preferences.Name = ChannelMembers.ChannelId and Preferences.UserId = ChannelMembers.UserId").
+		Where(sq.Eq{
+			"Preferences.UserId":   userId,
+			"Preferences.Category": model.PREFERENCE_CATEGORY_FAVORITE_CHANNEL,
+			"Preferences.Value":    "true",
+		}).
+		Where(sq.Or{
+			sq.Eq{"Channels.TeamId": teamId},
+			sq.Eq{"Channels.TeamId": ""},
+		}).
+		OrderBy(
+			"Channels.DisplayName",
+			"Channels.Name ASC",
+		).ToSql()
+
+	var favoriteChannelIds []string
+	if _, err := transaction.Select(&favoriteChannelIds, favoritesQuery, favoritesParams...); err != nil {
+		return errors.Wrap(err, "migrateFavoritesToSidebarT: unable to get favorite channel IDs")
+	}
+
+	for i, channelId := range favoriteChannelIds {
+		if err := transaction.Insert(&model.SidebarChannel{
+			ChannelId:  channelId,
+			CategoryId: favoritesCategoryId,
+			UserId:     userId,
+			SortOrder:  int64(i * model.MinimalSidebarSortDistance),
+		}); err != nil {
+			return errors.Wrap(err, "migrateFavoritesToSidebarT: unable to insert SidebarChannel")
+		}
+	}
+
+	return nil
+}
+
 // MigrateFavoritesToSidebarChannels populates the SidebarChannels table by analyzing existing user preferences for favorites
 // **IMPORTANT** This function should only be called from the migration task and shouldn't be used by itself
 func (s SqlChannelStore) MigrateFavoritesToSidebarChannels(lastUserId string, runningOrder int64) (map[string]interface{}, error) {

store/store.go

@@ -216,9 +216,7 @@ type ChannelStore interface {
 	ResetAllChannelSchemes() *model.AppError
 	ClearAllCustomRoleAssignments() *model.AppError
 	MigratePublicChannels() error
-	MigrateSidebarCategories(fromTeamId, fromUserId string) (map[string]interface{}, error)
 	CreateInitialSidebarCategories(userId, teamId string) error
-	MigrateFavoritesToSidebarChannels(lastUserId string, runningOrder int64) (map[string]interface{}, error)
 	GetSidebarCategories(userId, teamId string) (*model.OrderedSidebarCategories, *model.AppError)
 	GetSidebarCategory(categoryId string) (*model.SidebarCategoryWithChannels, *model.AppError)
 	GetSidebarCategoryOrder(userId, teamId string) ([]string, *model.AppError)

store/storetest/channel_store.go

@@ -101,7 +101,6 @@ func TestChannelStore(t *testing.T, ss store.Store, s SqlSupplier) {
 	t.Run("ExportAllDirectChannelsDeletedChannel", func(t *testing.T) { testChannelStoreExportAllDirectChannelsDeletedChannel(t, ss, s) })
 	t.Run("GetChannelsBatchForIndexing", func(t *testing.T) { testChannelStoreGetChannelsBatchForIndexing(t, ss) })
 	t.Run("GroupSyncedChannelCount", func(t *testing.T) { testGroupSyncedChannelCount(t, ss) })
-	t.Run("SidebarChannelsMigration", func(t *testing.T) { testSidebarChannelsMigration(t, ss) })
 	t.Run("CreateInitialSidebarCategories", func(t *testing.T) { testCreateInitialSidebarCategories(t, ss) })
 	t.Run("GetSidebarCategory", func(t *testing.T) { testGetSidebarCategory(t, ss, s) })
 	t.Run("GetSidebarCategories", func(t *testing.T) { testGetSidebarCategories(t, ss) })
@@ -6708,136 +6707,6 @@ func testGroupSyncedChannelCount(t *testing.T, ss store.Store) {
 	require.GreaterOrEqual(t, countAfter, count+1)
 }
 
-func testSidebarChannelsMigration(t *testing.T, ss store.Store) {
-	teamId := model.NewId()
-
-	channel1, err := ss.Channel().Save(&model.Channel{
-		DisplayName:      model.NewId(),
-		Name:             model.NewId(),
-		TeamId:           teamId,
-		Type:             model.CHANNEL_PRIVATE,
-		GroupConstrained: model.NewBool(true),
-	}, 10)
-	require.Nil(t, err)
-	defer func() {
-		ss.Channel().PermanentDeleteMembersByChannel(channel1.Id)
-		ss.Channel().PermanentDeleteByTeam(teamId)
-		ss.Channel().PermanentDelete(channel1.Id)
-	}()
-
-	channel2, err := ss.Channel().Save(&model.Channel{
-		DisplayName:      model.NewId(),
-		Name:             model.NewId(),
-		TeamId:           teamId,
-		Type:             model.CHANNEL_PRIVATE,
-		GroupConstrained: model.NewBool(true),
-	}, 10)
-	require.Nil(t, err)
-	defer func() {
-		ss.Channel().PermanentDeleteMembersByChannel(channel2.Id)
-		ss.Channel().PermanentDeleteByTeam(teamId)
-		ss.Channel().PermanentDelete(channel2.Id)
-	}()
-
-	var users []*model.User
-	for i := 0; i < 3; i++ {
-		u := &model.User{Email: MakeEmail(), Nickname: model.NewId()}
-		_, err = ss.User().Save(u)
-		require.Nil(t, err)
-		_, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u.Id}, -1)
-		require.Nil(t, err)
-		users = append(users, u)
-	}
-
-	_, err = ss.Channel().SaveMember(&model.ChannelMember{
-		ChannelId:   channel1.Id,
-		UserId:      users[0].Id,
-		NotifyProps: model.GetDefaultChannelNotifyProps(),
-	})
-	require.Nil(t, err)
-
-	_, err = ss.Channel().SaveMember(&model.ChannelMember{
-		ChannelId:   channel2.Id,
-		UserId:      users[0].Id,
-		NotifyProps: model.GetDefaultChannelNotifyProps(),
-	})
-	require.Nil(t, err)
-
-	err = ss.Preference().Save(&model.Preferences{
-		{
-			Category: model.PREFERENCE_CATEGORY_FAVORITE_CHANNEL,
-			Name:     channel1.Id,
-			UserId:   users[0].Id,
-			Value:    "true",
-		},
-	})
-	require.Nil(t, err)
-
-	_, err = ss.Channel().CreateDirectChannel(users[0], users[1])
-	require.Nil(t, err)
-
-	t.Run("MigrateSidebarCategories", func(t *testing.T) {
-		_, nErr := ss.Channel().MigrateSidebarCategories(strings.Repeat("0", 26), strings.Repeat("0", 26))
-		require.Nil(t, nErr)
-		res, err2 := ss.Channel().GetSidebarCategories(users[0].Id, teamId)
-		require.Nil(t, err2)
-		require.Len(t, res.Categories, 3)
-		require.Equal(t, model.SidebarCategoryFavorites, res.Categories[0].Type)
-		require.Equal(t, model.SidebarCategoryChannels, res.Categories[1].Type)
-		require.Equal(t, model.SidebarCategoryDirectMessages, res.Categories[2].Type)
-	})
-
-	t.Run("MigrateFavoritesToSidebarChannels", func(t *testing.T) {
-		_, nErr := ss.Channel().MigrateFavoritesToSidebarChannels(strings.Repeat("0", 26), 0)
-		require.Nil(t, nErr)
-	})
-
-	t.Run("GetSidebarCategories", func(t *testing.T) {
-		res, err := ss.Channel().GetSidebarCategories(users[0].Id, teamId)
-		require.Nil(t, err)
-		require.Equal(t, model.SidebarCategoryFavorites, res.Categories[0].Type)
-		require.Len(t, res.Categories[0].Channels, 1)
-		require.Equal(t, model.SidebarCategoryChannels, res.Categories[1].Type)
-		require.Len(t, res.Categories[1].Channels, 1)
-		require.Equal(t, model.SidebarCategoryDirectMessages, res.Categories[2].Type)
-		require.Len(t, res.Categories[2].Channels, 1)
-	})
-
-	t.Run("GetSidebarCategoriesWithoutNewChannel", func(t *testing.T) {
-		channel3, err := ss.Channel().Save(&model.Channel{
-			DisplayName:      model.NewId(),
-			Name:             model.NewId(),
-			TeamId:           teamId,
-			Type:             model.CHANNEL_PRIVATE,
-			GroupConstrained: model.NewBool(true),
-		}, 10)
-		require.Nil(t, err)
-		channel4, err := ss.Channel().CreateDirectChannel(users[0], users[2])
-		require.Nil(t, err)
-
-		defer func() {
-			ss.Channel().PermanentDeleteMembersByChannel(channel3.Id)
-			ss.Channel().PermanentDelete(channel3.Id)
-			ss.Channel().PermanentDeleteMembersByChannel(channel4.Id)
-			ss.Channel().PermanentDelete(channel4.Id)
-			ss.Channel().PermanentDeleteByTeam(teamId)
-		}()
-
-		_, err = ss.Channel().SaveMember(&model.ChannelMember{
-			ChannelId:   channel3.Id,
-			UserId:      users[0].Id,
-			NotifyProps: model.GetDefaultChannelNotifyProps(),
-		})
-		require.Nil(t, err)
-
-		res, err := ss.Channel().GetSidebarCategories(users[0].Id, teamId)
-		require.Nil(t, err)
-		require.Len(t, res.Categories[0].Channels, 1)
-		require.Len(t, res.Categories[1].Channels, 2)
-		require.Len(t, res.Categories[2].Channels, 2)
-	})
-}
-
 func testGetSidebarCategory(t *testing.T, ss store.Store, s SqlSupplier) {
 	t.Run("should return a custom category with its Channels field set", func(t *testing.T) {
 		userId := model.NewId()
@@ -7984,6 +7853,229 @@ func testCreateInitialSidebarCategories(t *testing.T, ss store.Store) {
 		assert.Nil(t, err)
 		assert.Equal(t, initialCategories.Categories, res.Categories)
 	})
+
+	t.Run("should populate the Favorites category with regular channels", func(t *testing.T) {
+		userId := model.NewId()
+		teamId := model.NewId()
+
+		// Set up two channels, one favorited and one not
+		channel1, nErr := ss.Channel().Save(&model.Channel{
+			TeamId: teamId,
+			Type:   model.CHANNEL_OPEN,
+			Name:   "channel1",
+		}, 1000)
+		require.Nil(t, nErr)
+		_, err := ss.Channel().SaveMember(&model.ChannelMember{
+			ChannelId:   channel1.Id,
+			UserId:      userId,
+			NotifyProps: model.GetDefaultChannelNotifyProps(),
+		})
+		require.Nil(t, err)
+
+		channel2, nErr := ss.Channel().Save(&model.Channel{
+			TeamId: teamId,
+			Type:   model.CHANNEL_OPEN,
+			Name:   "channel2",
+		}, 1000)
+		require.Nil(t, nErr)
+		_, err = ss.Channel().SaveMember(&model.ChannelMember{
+			ChannelId:   channel2.Id,
+			UserId:      userId,
+			NotifyProps: model.GetDefaultChannelNotifyProps(),
+		})
+		require.Nil(t, err)
+
+		err = ss.Preference().Save(&model.Preferences{
+			{
+				UserId:   userId,
+				Category: model.PREFERENCE_CATEGORY_FAVORITE_CHANNEL,
+				Name:     channel1.Id,
+				Value:    "true",
+			},
+		})
+		require.Nil(t, err)
+
+		// Create the categories
+		nErr = ss.Channel().CreateInitialSidebarCategories(userId, teamId)
+		require.Nil(t, nErr)
+
+		// Get and check the categories for channels
+		categories, err := ss.Channel().GetSidebarCategories(userId, teamId)
+		require.Nil(t, err)
+		require.Len(t, categories.Categories, 3)
+		assert.Equal(t, model.SidebarCategoryFavorites, categories.Categories[0].Type)
+		assert.Equal(t, []string{channel1.Id}, categories.Categories[0].Channels)
+		assert.Equal(t, model.SidebarCategoryChannels, categories.Categories[1].Type)
+		assert.Equal(t, []string{channel2.Id}, categories.Categories[1].Channels)
+	})
+
+	t.Run("should populate the Favorites category in alphabetical order", func(t *testing.T) {
+		userId := model.NewId()
+		teamId := model.NewId()
+
+		// Set up two channels
+		channel1, nErr := ss.Channel().Save(&model.Channel{
+			TeamId:      teamId,
+			Type:        model.CHANNEL_OPEN,
+			Name:        "channel1",
+			DisplayName: "zebra",
+		}, 1000)
+		require.Nil(t, nErr)
+		_, err := ss.Channel().SaveMember(&model.ChannelMember{
+			ChannelId:   channel1.Id,
+			UserId:      userId,
+			NotifyProps: model.GetDefaultChannelNotifyProps(),
+		})
+		require.Nil(t, err)
+
+		channel2, nErr := ss.Channel().Save(&model.Channel{
+			TeamId:      teamId,
+			Type:        model.CHANNEL_OPEN,
+			Name:        "channel2",
+			DisplayName: "aardvark",
+		}, 1000)
+		require.Nil(t, nErr)
+		_, err = ss.Channel().SaveMember(&model.ChannelMember{
+			ChannelId:   channel2.Id,
+			UserId:      userId,
+			NotifyProps: model.GetDefaultChannelNotifyProps(),
+		})
+		require.Nil(t, err)
+
+		err = ss.Preference().Save(&model.Preferences{
+			{
+				UserId:   userId,
+				Category: model.PREFERENCE_CATEGORY_FAVORITE_CHANNEL,
+				Name:     channel1.Id,
+				Value:    "true",
+			},
+			{
+				UserId:   userId,
+				Category: model.PREFERENCE_CATEGORY_FAVORITE_CHANNEL,
+				Name:     channel2.Id,
+				Value:    "true",
+			},
+		})
+		require.Nil(t, err)
+
+		// Create the categories
+		nErr = ss.Channel().CreateInitialSidebarCategories(userId, teamId)
+		require.Nil(t, nErr)
+
+		// Get and check the categories for channels
+		categories, err := ss.Channel().GetSidebarCategories(userId, teamId)
+		require.Nil(t, err)
+		require.Len(t, categories.Categories, 3)
+		assert.Equal(t, model.SidebarCategoryFavorites, categories.Categories[0].Type)
+		assert.Equal(t, []string{channel2.Id, channel1.Id}, categories.Categories[0].Channels)
+	})
+
+	t.Run("should populate the Favorites category with DMs and GMs", func(t *testing.T) {
+		userId := model.NewId()
+		teamId := model.NewId()
+
+		otherUserId1 := model.NewId()
+		otherUserId2 := model.NewId()
+
+		// Set up two direct channels, one favorited and one not
+		dmChannel1, err := ss.Channel().SaveDirectChannel(
+			&model.Channel{
+				Name: model.GetDMNameFromIds(userId, otherUserId1),
+				Type: model.CHANNEL_DIRECT,
+			},
+			&model.ChannelMember{
+				UserId:      userId,
+				NotifyProps: model.GetDefaultChannelNotifyProps(),
+			},
+			&model.ChannelMember{
+				UserId:      otherUserId1,
+				NotifyProps: model.GetDefaultChannelNotifyProps(),
+			},
+		)
+		require.Nil(t, err)
+
+		dmChannel2, err := ss.Channel().SaveDirectChannel(
+			&model.Channel{
+				Name: model.GetDMNameFromIds(userId, otherUserId2),
+				Type: model.CHANNEL_DIRECT,
+			},
+			&model.ChannelMember{
+				UserId:      userId,
+				NotifyProps: model.GetDefaultChannelNotifyProps(),
+			},
+			&model.ChannelMember{
+				UserId:      otherUserId2,
+				NotifyProps: model.GetDefaultChannelNotifyProps(),
+			},
+		)
+		require.Nil(t, err)
+
+		err = ss.Preference().Save(&model.Preferences{
+			{
+				UserId:   userId,
+				Category: model.PREFERENCE_CATEGORY_FAVORITE_CHANNEL,
+				Name:     dmChannel1.Id,
+				Value:    "true",
+			},
+		})
+		require.Nil(t, err)
+
+		// Create the categories
+		nErr := ss.Channel().CreateInitialSidebarCategories(userId, teamId)
+		require.Nil(t, nErr)
+
+		// Get and check the categories for channels
+		categories, err := ss.Channel().GetSidebarCategories(userId, teamId)
+		require.Nil(t, err)
+		require.Len(t, categories.Categories, 3)
+		assert.Equal(t, model.SidebarCategoryFavorites, categories.Categories[0].Type)
+		assert.Equal(t, []string{dmChannel1.Id}, categories.Categories[0].Channels)
+		assert.Equal(t, model.SidebarCategoryDirectMessages, categories.Categories[2].Type)
+		assert.Equal(t, []string{dmChannel2.Id}, categories.Categories[2].Channels)
+	})
+
+	t.Run("should not populate the Favorites category with channels from other teams", func(t *testing.T) {
+		userId := model.NewId()
+		teamId := model.NewId()
+		teamId2 := model.NewId()
+
+		// Set up a channel on another team and favorite it
+		channel1, nErr := ss.Channel().Save(&model.Channel{
+			TeamId: teamId2,
+			Type:   model.CHANNEL_OPEN,
+			Name:   "channel1",
+		}, 1000)
+		require.Nil(t, nErr)
+		_, err := ss.Channel().SaveMember(&model.ChannelMember{
+			ChannelId:   channel1.Id,
+			UserId:      userId,
+			NotifyProps: model.GetDefaultChannelNotifyProps(),
+		})
+		require.Nil(t, err)
+
+		err = ss.Preference().Save(&model.Preferences{
+			{
+				UserId:   userId,
+				Category: model.PREFERENCE_CATEGORY_FAVORITE_CHANNEL,
+				Name:     channel1.Id,
+				Value:    "true",
+			},
+		})
+		require.Nil(t, err)
+
+		// Create the categories
+		nErr = ss.Channel().CreateInitialSidebarCategories(userId, teamId)
+		require.Nil(t, nErr)
+
+		// Get and check the categories for channels
+		categories, err := ss.Channel().GetSidebarCategories(userId, teamId)
+		require.Nil(t, err)
+		require.Len(t, categories.Categories, 3)
+		assert.Equal(t, model.SidebarCategoryFavorites, categories.Categories[0].Type)
+		assert.Equal(t, []string{}, categories.Categories[0].Channels)
+		assert.Equal(t, model.SidebarCategoryChannels, categories.Categories[1].Type)
+		assert.Equal(t, []string{}, categories.Categories[1].Channels)
+	})
 }
 
 func testDeleteSidebarCategory(t *testing.T, ss store.Store, s SqlSupplier) {

store/storetest/mocks/ChannelStore.go

@@ -1424,29 +1424,6 @@ func (_m *ChannelStore) MigrateChannelMembers(fromChannelId string, fromUserId s
 	return r0, r1
 }
 
-// MigrateFavoritesToSidebarChannels provides a mock function with given fields: lastUserId, runningOrder
-func (_m *ChannelStore) MigrateFavoritesToSidebarChannels(lastUserId string, runningOrder int64) (map[string]interface{}, error) {
-	ret := _m.Called(lastUserId, runningOrder)
-
-	var r0 map[string]interface{}
-	if rf, ok := ret.Get(0).(func(string, int64) map[string]interface{}); ok {
-		r0 = rf(lastUserId, runningOrder)
-	} else {
-		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(map[string]interface{})
-		}
-	}
-
-	var r1 error
-	if rf, ok := ret.Get(1).(func(string, int64) error); ok {
-		r1 = rf(lastUserId, runningOrder)
-	} else {
-		r1 = ret.Error(1)
-	}
-
-	return r0, r1
-}
-
 // MigratePublicChannels provides a mock function with given fields:
 func (_m *ChannelStore) MigratePublicChannels() error {
 	ret := _m.Called()
@@ -1461,29 +1438,6 @@ func (_m *ChannelStore) MigratePublicChannels() error {
 	return r0
 }
 
-// MigrateSidebarCategories provides a mock function with given fields: fromTeamId, fromUserId
-func (_m *ChannelStore) MigrateSidebarCategories(fromTeamId string, fromUserId string) (map[string]interface{}, error) {
-	ret := _m.Called(fromTeamId, fromUserId)
-
-	var r0 map[string]interface{}
-	if rf, ok := ret.Get(0).(func(string, string) map[string]interface{}); ok {
-		r0 = rf(fromTeamId, fromUserId)
-	} else {
-		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(map[string]interface{})
-		}
-	}
-
-	var r1 error
-	if rf, ok := ret.Get(1).(func(string, string) error); ok {
-		r1 = rf(fromTeamId, fromUserId)
-	} else {
-		r1 = ret.Error(1)
-	}
-
-	return r0, r1
-}
-
 // PermanentDelete provides a mock function with given fields: channelId
 func (_m *ChannelStore) PermanentDelete(channelId string) error {
 	ret := _m.Called(channelId)

store/storetest/mocks/CommandWebhookStore.go

@@ -36,9 +36,7 @@ func (_m *CommandWebhookStore) Get(id string) (*model.CommandWebhook, error) {
 	if rf, ok := ret.Get(1).(func(string) error); ok {
 		r1 = rf(id)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(error)
-		}
 	}
 
 	return r0, r1
@@ -61,9 +59,7 @@ func (_m *CommandWebhookStore) Save(webhook *model.CommandWebhook) (*model.Comma
 	if rf, ok := ret.Get(1).(func(*model.CommandWebhook) error); ok {
 		r1 = rf(webhook)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(error)
-		}
 	}
 
 	return r0, r1
@@ -77,9 +73,7 @@ func (_m *CommandWebhookStore) TryUse(id string, limit int) error {
 	if rf, ok := ret.Get(0).(func(string, int) error); ok {
 		r0 = rf(id, limit)
 	} else {
-		if ret.Get(0) != nil {
+		r0 = ret.Error(0)
-			r0 = ret.Get(0).(error)
-		}
 	}
 
 	return r0

store/timer_layer.go

@@ -1537,22 +1537,6 @@ func (s *TimerLayerChannelStore) MigrateChannelMembers(fromChannelId string, fro
 	return resultVar0, resultVar1
 }
 
-func (s *TimerLayerChannelStore) MigrateFavoritesToSidebarChannels(lastUserId string, runningOrder int64) (map[string]interface{}, error) {
-	start := timemodule.Now()
-
-	resultVar0, resultVar1 := s.ChannelStore.MigrateFavoritesToSidebarChannels(lastUserId, runningOrder)
-
-	elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second)
-	if s.Root.Metrics != nil {
-		success := "false"
-		if resultVar1 == nil {
-			success = "true"
-		}
-		s.Root.Metrics.ObserveStoreMethodDuration("ChannelStore.MigrateFavoritesToSidebarChannels", success, elapsed)
-	}
-	return resultVar0, resultVar1
-}
-
 func (s *TimerLayerChannelStore) MigratePublicChannels() error {
 	start := timemodule.Now()
 
@@ -1569,22 +1553,6 @@ func (s *TimerLayerChannelStore) MigratePublicChannels() error {
 	return resultVar0
 }
 
-func (s *TimerLayerChannelStore) MigrateSidebarCategories(fromTeamId string, fromUserId string) (map[string]interface{}, error) {
-	start := timemodule.Now()
-
-	resultVar0, resultVar1 := s.ChannelStore.MigrateSidebarCategories(fromTeamId, fromUserId)
-
-	elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second)
-	if s.Root.Metrics != nil {
-		success := "false"
-		if resultVar1 == nil {
-			success = "true"
-		}
-		s.Root.Metrics.ObserveStoreMethodDuration("ChannelStore.MigrateSidebarCategories", success, elapsed)
-	}
-	return resultVar0, resultVar1
-}
-
 func (s *TimerLayerChannelStore) PermanentDelete(channelId string) error {
 	start := timemodule.Now()