Merge pull request #4625 from mattermost/cp-4614

Merge pull request #4614 from mattermost/deduplicate_category_boards

mattermost-plugin/server/manifest.go

@@ -45,8 +45,7 @@ const manifestStr = `
         "type": "bool",
         "help_text": "This allows board editors to share boards that can be accessed by anyone with the link.",
         "placeholder": "",
-        "default": false,
+        "default": false
-        "hosting": ""
       }
     ]
   }

server/services/store/sqlstore/data_migrations.go

@@ -21,11 +21,12 @@ const (
 	// query, so we want to stay safely below.
 	CategoryInsertBatch = 1000
 
-	TemplatesToTeamsMigrationKey        = "TemplatesToTeamsMigrationComplete"
+	TemplatesToTeamsMigrationKey              = "TemplatesToTeamsMigrationComplete"
-	UniqueIDsMigrationKey               = "UniqueIDsMigrationComplete"
+	UniqueIDsMigrationKey                     = "UniqueIDsMigrationComplete"
-	CategoryUUIDIDMigrationKey          = "CategoryUuidIdMigrationComplete"
+	CategoryUUIDIDMigrationKey                = "CategoryUuidIdMigrationComplete"
-	TeamLessBoardsMigrationKey          = "TeamLessBoardsMigrationComplete"
+	TeamLessBoardsMigrationKey                = "TeamLessBoardsMigrationComplete"
-	DeletedMembershipBoardsMigrationKey = "DeletedMembershipBoardsMigrationComplete"
+	DeletedMembershipBoardsMigrationKey       = "DeletedMembershipBoardsMigrationComplete"
+	DeDuplicateCategoryBoardTableMigrationKey = "DeDuplicateCategoryBoardTableComplete"
 )
 
 func (s *SQLStore) getBlocksWithSameID(db sq.BaseRunner) ([]*model.Block, error) {
@@ -790,3 +791,102 @@ func (s *SQLStore) getCollationAndCharset(tableName string) (string, string, err
 
 	return collation, charSet, nil
 }
+
+func (s *SQLStore) RunDeDuplicateCategoryBoardsMigration(currentMigration int) error {
+	// not supported for SQLite
+	if s.dbType == model.SqliteDBType {
+		if mErr := s.setSystemSetting(s.db, DeDuplicateCategoryBoardTableMigrationKey, strconv.FormatBool(true)); mErr != nil {
+			return fmt.Errorf("cannot mark migration %s as completed: %w", "RunDeDuplicateCategoryBoardsMigration", mErr)
+		}
+		return nil
+	}
+
+	setting, err := s.GetSystemSetting(DeDuplicateCategoryBoardTableMigrationKey)
+	if err != nil {
+		return fmt.Errorf("cannot get DeDuplicateCategoryBoardTableMigration state: %w", err)
+	}
+
+	// If the migration is already completed, do not run it again.
+	if hasAlreadyRun, _ := strconv.ParseBool(setting); hasAlreadyRun {
+		return nil
+	}
+
+	if currentMigration >= (deDuplicateCategoryBoards + 1) {
+		// if the migration for which we're fixing the data is already applied,
+		// no need to check fix anything
+
+		if mErr := s.setSystemSetting(s.db, DeDuplicateCategoryBoardTableMigrationKey, strconv.FormatBool(true)); mErr != nil {
+			return fmt.Errorf("cannot mark migration %s as completed: %w", "RunDeDuplicateCategoryBoardsMigration", mErr)
+		}
+		return nil
+	}
+
+	needed, err := s.doesDuplicateCategoryBoardsExist()
+	if err != nil {
+		return err
+	}
+
+	if !needed {
+		if mErr := s.setSystemSetting(s.db, DeDuplicateCategoryBoardTableMigrationKey, strconv.FormatBool(true)); mErr != nil {
+			return fmt.Errorf("cannot mark migration %s as completed: %w", "RunDeDuplicateCategoryBoardsMigration", mErr)
+		}
+	}
+
+	if s.dbType == model.MysqlDBType {
+		return s.runMySQLDeDuplicateCategoryBoardsMigration()
+	} else if s.dbType == model.PostgresDBType {
+		return s.runPostgresDeDuplicateCategoryBoardsMigration()
+	}
+
+	if mErr := s.setSystemSetting(s.db, DeDuplicateCategoryBoardTableMigrationKey, strconv.FormatBool(true)); mErr != nil {
+		return fmt.Errorf("cannot mark migration %s as completed: %w", "RunDeDuplicateCategoryBoardsMigration", mErr)
+	}
+
+	return nil
+}
+
+func (s *SQLStore) doesDuplicateCategoryBoardsExist() (bool, error) {
+	subQuery := s.getQueryBuilder(s.db).
+		Select("user_id", "board_id", "count(*) AS count").
+		From(s.tablePrefix+"category_boards").
+		GroupBy("user_id", "board_id").
+		Having("count(*) > 1")
+
+	query := s.getQueryBuilder(s.db).
+		Select("COUNT(user_id)").
+		FromSelect(subQuery, "duplicate_dataset")
+
+	row := query.QueryRow()
+
+	count := 0
+	if err := row.Scan(&count); err != nil {
+		s.logger.Error("Error occurred reading number of duplicate records in category_boards table", mlog.Err(err))
+		return false, err
+	}
+
+	return count > 0, nil
+}
+
+func (s *SQLStore) runMySQLDeDuplicateCategoryBoardsMigration() error {
+	query := "WITH duplicates AS (SELECT id, ROW_NUMBER() OVER(PARTITION BY user_id, board_id) AS rownum " +
+		"FROM " + s.tablePrefix + "category_boards) " +
+		"DELETE " + s.tablePrefix + "category_boards FROM " + s.tablePrefix + "category_boards " +
+		"JOIN duplicates USING(id) WHERE duplicates.rownum > 1;"
+	if _, err := s.db.Exec(query); err != nil {
+		s.logger.Error("Failed to de-duplicate data in category_boards table", mlog.Err(err))
+	}
+
+	return nil
+}
+
+func (s *SQLStore) runPostgresDeDuplicateCategoryBoardsMigration() error {
+	query := "WITH duplicates AS (SELECT id, ROW_NUMBER() OVER(PARTITION BY user_id, board_id) AS rownum " +
+		"FROM " + s.tablePrefix + "category_boards) " +
+		"DELETE FROM " + s.tablePrefix + "category_boards USING duplicates " +
+		"WHERE " + s.tablePrefix + "category_boards.id = duplicates.id AND duplicates.rownum > 1;"
+	if _, err := s.db.Exec(query); err != nil {
+		s.logger.Error("Failed to de-duplicate data in category_boards table", mlog.Err(err))
+	}
+
+	return nil
+}

server/services/store/sqlstore/migrate.go

@@ -36,6 +36,7 @@ const (
 	uniqueIDsMigrationRequiredVersion        = 14
 	teamLessBoardsMigrationRequiredVersion   = 18
 	categoriesUUIDIDMigrationRequiredVersion = 20
+	deDuplicateCategoryBoards                = 35
 
 	tempSchemaMigrationTableName = "temp_schema_migration"
 )
@@ -248,6 +249,15 @@ func (s *SQLStore) runMigrationSequence(engine *morph.Morph, driver drivers.Driv
 		return err
 	}
 
+	if mErr := s.ensureMigrationsAppliedUpToVersion(engine, driver, deDuplicateCategoryBoards); mErr != nil {
+		return mErr
+	}
+
+	currentMigrationVersion := len(appliedMigrations)
+	if mErr := s.RunDeDuplicateCategoryBoardsMigration(currentMigrationVersion); mErr != nil {
+		return mErr
+	}
+
 	s.logger.Debug("== Applying all remaining migrations ====================",
 		mlog.Int("current_version", len(appliedMigrations)),
 	)

server/services/store/storetests/system.go

@@ -10,8 +10,9 @@ import (
 // these system settings are created when running the data migrations,
 // so they will be present after the tests setup.
 var dataMigrationSystemSettings = map[string]string{
-	"UniqueIDsMigrationComplete":      "true",
+	"UniqueIDsMigrationComplete":            "true",
-	"CategoryUuidIdMigrationComplete": "true",
+	"CategoryUuidIdMigrationComplete":       "true",
+	"DeDuplicateCategoryBoardTableComplete": "true",
 }
 
 func addBaseSettings(m map[string]string) map[string]string {