diff --git a/mattermost-plugin/.golangci.yml b/mattermost-plugin/.golangci.yml index e8163d76b..90dbb3596 100644 --- a/mattermost-plugin/.golangci.yml +++ b/mattermost-plugin/.golangci.yml @@ -67,7 +67,6 @@ linters: - unconvert - unused - whitespace - - gocyclo issues: exclude-rules: diff --git a/mattermost-plugin/server/plugin.go b/mattermost-plugin/server/plugin.go index 39febf012..e91c87ba1 100644 --- a/mattermost-plugin/server/plugin.go +++ b/mattermost-plugin/server/plugin.go @@ -99,6 +99,7 @@ func (p *Plugin) OnActivate() error { NewMutexFn: func(name string) (*cluster.Mutex, error) { return cluster.NewMutex(p.API, name) }, + PluginAPI: &p.API, } var db store.Store diff --git a/server/.golangci.yml b/server/.golangci.yml index c1381759f..b336a462a 100644 --- a/server/.golangci.yml +++ b/server/.golangci.yml @@ -65,4 +65,3 @@ linters: - unconvert - unused - whitespace - - gocyclo diff --git a/server/services/store/sqlstore/board.go b/server/services/store/sqlstore/board.go index 421fab52c..17e375a4b 100644 --- a/server/services/store/sqlstore/board.go +++ b/server/services/store/sqlstore/board.go @@ -213,14 +213,34 @@ func (s *SQLStore) getBoardsForUserAndTeam(db sq.BaseRunner, userID, teamID stri func (s *SQLStore) insertBoard(db sq.BaseRunner, board *model.Board, userID string) (*model.Board, error) { propertiesBytes, err := json.Marshal(board.Properties) if err != nil { + s.logger.Error( + "failed to marshal board.Properties", + mlog.String("board_id", board.ID), + mlog.String("board.Properties", fmt.Sprintf("%v", board.Properties)), + mlog.Err(err), + ) return nil, err } + cardPropertiesBytes, err := json.Marshal(board.CardProperties) if err != nil { + s.logger.Error( + "failed to marshal board.CardProperties", + mlog.String("board_id", board.ID), + mlog.String("board.CardProperties", fmt.Sprintf("%v", board.CardProperties)), + mlog.Err(err), + ) return nil, err } + columnCalculationsBytes, err := json.Marshal(board.ColumnCalculations) if err != nil { + s.logger.Error( + "failed to marshal board.ColumnCalculations", + mlog.String("board_id", board.ID), + mlog.String("board.ColumnCalculations", fmt.Sprintf("%v", board.ColumnCalculations)), + mlog.Err(err), + ) return nil, err } @@ -290,6 +310,7 @@ func (s *SQLStore) insertBoard(db sq.BaseRunner, board *model.Board, userID stri // writing board history query := insertQuery.SetMap(insertQueryValues).Into(s.tablePrefix + "boards_history") if _, err := query.Exec(); err != nil { + s.logger.Error("failed to insert board history", mlog.String("board_id", board.ID), mlog.Err(err)) return nil, err } @@ -395,7 +416,7 @@ func (s *SQLStore) saveMember(db sq.BaseRunner, bm *model.BoardMember) (*model.B } else { query = query.Suffix( `ON CONFLICT (board_id, user_id) - DO UPDATE SET scheme_admin = EXCLUDED.scheme_admin, scheme_editor = EXCLUDED.scheme_editor, + DO UPDATE SET scheme_admin = EXCLUDED.scheme_admin, scheme_editor = EXCLUDED.scheme_editor, scheme_commenter = EXCLUDED.scheme_commenter, scheme_viewer = EXCLUDED.scheme_viewer`, ) } diff --git a/server/services/store/sqlstore/migrate.go b/server/services/store/sqlstore/migrate.go index 42bb7390d..63b681403 100644 --- a/server/services/store/sqlstore/migrate.go +++ b/server/services/store/sqlstore/migrate.go @@ -5,8 +5,13 @@ import ( "context" "database/sql" "embed" + "errors" "fmt" + + "github.com/mattermost/focalboard/server/utils" + "path/filepath" + "strconv" "text/template" "github.com/mattermost/morph/models" @@ -33,11 +38,16 @@ import ( var assets embed.FS const ( - uniqueIDsMigrationRequiredVersion = 14 + uniqueIDsMigrationRequiredVersion = 14 + teamsAndBoardsMigrationRequiredVersion = 17 + + teamLessBoardsMigrationKey = "TeamLessBoardsMigrationComplete" tempSchemaMigrationTableName = "temp_schema_migration" ) +var errChannelCreatorNotInTeam = errors.New("channel creator not found in user teams") + func appendMultipleStatementsFlag(connectionString string) (string, error) { config, err := mysqldriver.ParseDSN(connectionString) if err != nil { @@ -219,6 +229,23 @@ func (s *SQLStore) Migrate() error { } if err := s.deleteOldSchemaMigrationTable(); err != nil { + if s.isPlugin { + mutex.Unlock() + } + return err + } + + if err := ensureMigrationsAppliedUpToVersion(engine, driver, teamsAndBoardsMigrationRequiredVersion); err != nil { + if s.isPlugin { + mutex.Unlock() + } + return err + } + + if err := s.migrateTeamLessBoards(); err != nil { + if s.isPlugin { + mutex.Unlock() + } return err } @@ -441,6 +468,192 @@ func (s *SQLStore) deleteOldSchemaMigrationTable() error { return nil } +// We no longer support boards existing in DMs and private +// group messages. This function migrates all boards +// belonging to a DM to the best possible team. +func (s *SQLStore) migrateTeamLessBoards() error { + if !s.isPlugin { + return nil + } + + setting, err := s.GetSystemSetting(teamLessBoardsMigrationKey) + if err != nil { + return fmt.Errorf("cannot get teamless boards migration state: %w", err) + } + + // If the migration is already completed, do not run it again. + if hasAlreadyRun, _ := strconv.ParseBool(setting); hasAlreadyRun { + return nil + } + + boards, err := s.getDMBoards(s.db) + if err != nil { + return err + } + + s.logger.Info(fmt.Sprintf("Migrating %d teamless boards to a team", len(boards))) + + // cache for best suitable team for a DM. Since a DM can + // contain multiple boards, caching this avoids + // duplicate queries for the same DM. + channelToTeamCache := map[string]string{} + + tx, err := s.db.BeginTx(context.Background(), nil) + if err != nil { + s.logger.Error("error starting transaction in migrateTeamLessBoards", mlog.Err(err)) + return err + } + + for i := range boards { + // check the cache first + teamID, ok := channelToTeamCache[boards[i].ChannelID] + + // query DB if entry not found in cache + if !ok { + teamID, err = s.getBestTeamForBoard(s.db, boards[i]) + if err != nil { + // don't let one board's error spoil + // the mood for others + continue + } + } + + channelToTeamCache[boards[i].ChannelID] = teamID + boards[i].TeamID = teamID + + query := s.getQueryBuilder(tx). + Update(s.tablePrefix+"boards"). + Set("team_id", teamID). + Set("type", model.BoardTypePrivate). + Where(sq.Eq{"id": boards[i].ID}) + + if _, err := query.Exec(); err != nil { + s.logger.Error("failed to set team id for board", mlog.String("board_id", boards[i].ID), mlog.String("team_id", teamID), mlog.Err(err)) + return err + } + } + + if err := s.setSystemSetting(tx, teamLessBoardsMigrationKey, strconv.FormatBool(true)); err != nil { + if rollbackErr := tx.Rollback(); rollbackErr != nil { + s.logger.Error("transaction rollback error", mlog.Err(rollbackErr), mlog.String("methodName", "migrateTeamLessBoards")) + } + return fmt.Errorf("cannot mark migration as completed: %w", err) + } + + if err := tx.Commit(); err != nil { + s.logger.Error("failed to commit migrateTeamLessBoards transaction", mlog.Err(err)) + return err + } + + return nil +} + +func (s *SQLStore) getDMBoards(tx sq.BaseRunner) ([]*model.Board, error) { + conditions := sq.And{ + sq.Eq{"team_id": ""}, + sq.Or{ + sq.Eq{"type": "D"}, + sq.Eq{"type": "G"}, + }, + } + + return s.getBoardsByCondition(tx, conditions) +} + +// The destination is selected as the first team where all members +// of the DM are a part of. If no such team exists, +// we use the first team to which DM creator belongs to. +func (s *SQLStore) getBestTeamForBoard(tx sq.BaseRunner, board *model.Board) (string, error) { + userTeams, err := s.getBoardUserTeams(tx, board) + if err != nil { + return "", err + } + + teams := [][]interface{}{} + for _, userTeam := range userTeams { + userTeamInterfaces := make([]interface{}, len(userTeam)) + for i := range userTeam { + userTeamInterfaces[i] = userTeam[i] + } + teams = append(teams, userTeamInterfaces) + } + + commonTeams := utils.Intersection(teams...) + var teamID string + if len(commonTeams) > 0 { + teamID = commonTeams[0].(string) + } else { + // no common teams found. Let's try finding the best suitable team + if board.Type == "D" { + // get DM's creator and pick one of their team + channel, appErr := (*s.pluginAPI).GetChannel(board.ChannelID) + if appErr != nil { + s.logger.Error("failed to fetch DM channel for board", mlog.String("board_id", board.ID), mlog.String("channel_id", board.ChannelID), mlog.Err(appErr)) + return "", appErr + } + + if _, ok := userTeams[channel.CreatorId]; !ok { + err := fmt.Errorf("%w board_id: %s, channel_id: %s, creator_id: %s", errChannelCreatorNotInTeam, board.ID, board.ChannelID, channel.CreatorId) + s.logger.Error(err.Error()) + return "", err + } + + teamID = userTeams[channel.CreatorId][0] + } else if board.Type == "G" { + // pick the team that has the most users as members + teamFrequency := map[string]int{} + highestFrequencyTeam := "" + highestFrequencyTeamFrequency := -1 + + for _, teams := range userTeams { + for _, teamID := range teams { + teamFrequency[teamID]++ + + if teamFrequency[teamID] > highestFrequencyTeamFrequency { + highestFrequencyTeamFrequency = teamFrequency[teamID] + highestFrequencyTeam = teamID + } + } + } + + teamID = highestFrequencyTeam + } + } + + return teamID, nil +} + +func (s *SQLStore) getBoardUserTeams(tx sq.BaseRunner, board *model.Board) (map[string][]string, error) { + query := s.getQueryBuilder(tx). + Select("teammembers.userid", "teammembers.teamid"). + From("channelmembers"). + Join("teammembers ON channelmembers.userid = teammembers.userid"). + Where(sq.Eq{"channelid": board.ChannelID}) + + rows, err := query.Query() + if err != nil { + s.logger.Error("failed to fetch user teams for board", mlog.String("boardID", board.ID), mlog.String("channelID", board.ChannelID), mlog.Err(err)) + return nil, err + } + + defer rows.Close() + + userTeams := map[string][]string{} + + for rows.Next() { + var userID, teamID string + err := rows.Scan(&userID, &teamID) + if err != nil { + s.logger.Error("getBoardUserTeams failed to scan SQL query result", mlog.String("boardID", board.ID), mlog.String("channelID", board.ChannelID), mlog.Err(err)) + return nil, err + } + + userTeams[userID] = append(userTeams[userID], teamID) + } + + return userTeams, nil +} + func ensureMigrationsAppliedUpToVersion(engine *morph.Morph, driver drivers.Driver, version int) error { applied, err := driver.AppliedMigrations() if err != nil { diff --git a/server/services/store/sqlstore/migrations/000017_add_teams_and_boards.up.sql b/server/services/store/sqlstore/migrations/000017_add_teams_and_boards.up.sql index 5b31f86c2..ffd155a58 100644 --- a/server/services/store/sqlstore/migrations/000017_add_teams_and_boards.up.sql +++ b/server/services/store/sqlstore/migrations/000017_add_teams_and_boards.up.sql @@ -108,8 +108,10 @@ CREATE TABLE {{.prefix}}boards_history ( {{if .postgres}} INSERT INTO {{.prefix}}boards ( SELECT B.id, B.insert_at, C.TeamId, B.channel_id, B.created_by, B.modified_by, C.type, B.title, (B.fields->>'description')::text, - B.fields->>'icon', (B.fields->'showDescription')::text::boolean, (B.fields->'isTemplate')::text::boolean, - COALESCE((B.fields->'templateVer')::text, '0')::int, + B.fields->>'icon', + COALESCE((fields->'showDescription')::text::boolean, false), + COALESCE((fields->'isTemplate')::text::boolean, false), + COALESCE((B.fields->'templateVer')::text::int, 0), '{}', B.fields->'cardProperties', B.fields->'columnCalculations', B.create_at, B.update_at, B.delete_at FROM {{.prefix}}blocks AS B @@ -118,8 +120,10 @@ CREATE TABLE {{.prefix}}boards_history ( ); INSERT INTO {{.prefix}}boards_history ( SELECT B.id, B.insert_at, C.TeamId, B.channel_id, B.created_by, B.modified_by, C.type, B.title, (B.fields->>'description')::text, - B.fields->>'icon', (B.fields->'showDescription')::text::boolean, (B.fields->'isTemplate')::text::boolean, - COALESCE((B.fields->'templateVer')::text, '0')::int, + B.fields->>'icon', + COALESCE((fields->'showDescription')::text::boolean, false), + COALESCE((fields->'isTemplate')::text::boolean, false), + COALESCE((B.fields->'templateVer')::text::int, 0), '{}', B.fields->'cardProperties', B.fields->'columnCalculations', B.create_at, B.update_at, B.delete_at FROM {{.prefix}}blocks_history AS B @@ -157,8 +161,10 @@ CREATE TABLE {{.prefix}}boards_history ( {{if .postgres}} INSERT INTO {{.prefix}}boards ( SELECT id, insert_at, '0', channel_id, created_by, modified_by, 'O', title, (fields->>'description')::text, - B.fields->>'icon', (fields->'showDescription')::text::boolean, (fields->'isTemplate')::text::boolean, - (B.fields->'templateVer')::text::int, + B.fields->>'icon', + COALESCE((fields->'showDescription')::text::boolean, false), + COALESCE((fields->'isTemplate')::text::boolean, false), + COALESCE((B.fields->'templateVer')::text::int, 0), '{}', fields->'cardProperties', fields->'columnCalculations', create_at, update_at, delete_at FROM {{.prefix}}blocks AS B @@ -166,8 +172,10 @@ CREATE TABLE {{.prefix}}boards_history ( ); INSERT INTO {{.prefix}}boards_history ( SELECT id, insert_at, '0', channel_id, created_by, modified_by, 'O', title, (fields->>'description')::text, - B.fields->>'icon', (fields->'showDescription')::text::boolean, (fields->'isTemplate')::text::boolean, - (B.fields->'templateVer')::text::int, + B.fields->>'icon', + COALESCE((fields->'showDescription')::text::boolean, false), + COALESCE((fields->'isTemplate')::text::boolean, false), + COALESCE((B.fields->'templateVer')::text::int, 0), '{}', fields->'cardProperties', fields->'columnCalculations', create_at, update_at, delete_at FROM {{.prefix}}blocks_history AS B @@ -177,8 +185,10 @@ CREATE TABLE {{.prefix}}boards_history ( {{if .mysql}} INSERT INTO {{.prefix}}boards ( SELECT id, insert_at, '0', channel_id, created_by, modified_by, 'O', title, JSON_UNQUOTE(JSON_EXTRACT(fields,'$.description')), - JSON_UNQUOTE(JSON_EXTRACT(fields,'$.icon')), fields->'$.showDescription', fields->'$.isTemplate', - B.fields->'$.templateVer', + JSON_UNQUOTE(JSON_EXTRACT(fields,'$.icon')), + COALESCE(B.fields->'$.showDescription', 'false') = 'true', + COALESCE(JSON_EXTRACT(B.fields, '$.isTemplate'), 'false') = 'true', + COALESCE(B.fields->'$.templateVer', 0), '{}', fields->'$.cardProperties', fields->'$.columnCalculations', create_at, update_at, delete_at FROM {{.prefix}}blocks AS B @@ -186,8 +196,10 @@ CREATE TABLE {{.prefix}}boards_history ( ); INSERT INTO {{.prefix}}boards_history ( SELECT id, insert_at, '0', channel_id, created_by, modified_by, 'O', title, JSON_UNQUOTE(JSON_EXTRACT(fields,'$.description')), - JSON_UNQUOTE(JSON_EXTRACT(fields,'$.icon')), fields->'$.showDescription', fields->'$.isTemplate', - B.fields->'$.templateVer', + JSON_UNQUOTE(JSON_EXTRACT(fields,'$.icon')), + COALESCE(B.fields->'$.showDescription', 'false') = 'true', + COALESCE(JSON_EXTRACT(B.fields, '$.isTemplate'), 'false') = 'true', + COALESCE(B.fields->'$.templateVer', 0), '{}', fields->'$.cardProperties', fields->'$.columnCalculations', create_at, update_at, delete_at FROM {{.prefix}}blocks_history AS B diff --git a/server/services/store/sqlstore/params.go b/server/services/store/sqlstore/params.go index a6992f642..1a2590a08 100644 --- a/server/services/store/sqlstore/params.go +++ b/server/services/store/sqlstore/params.go @@ -4,6 +4,8 @@ import ( "database/sql" "fmt" + "github.com/mattermost/mattermost-server/v6/plugin" + "github.com/mattermost/mattermost-server/v6/shared/mlog" ) @@ -15,6 +17,7 @@ type Params struct { DB *sql.DB IsPlugin bool NewMutexFn MutexFactory + PluginAPI *plugin.API } func (p Params) CheckValid() error { diff --git a/server/services/store/sqlstore/sqlstore.go b/server/services/store/sqlstore/sqlstore.go index 297514683..6de84bff5 100644 --- a/server/services/store/sqlstore/sqlstore.go +++ b/server/services/store/sqlstore/sqlstore.go @@ -3,6 +3,8 @@ package sqlstore import ( "database/sql" + "github.com/mattermost/mattermost-server/v6/plugin" + sq "github.com/Masterminds/squirrel" "github.com/mattermost/focalboard/server/model" @@ -20,6 +22,7 @@ type SQLStore struct { isPlugin bool logger *mlog.Logger NewMutexFn MutexFactory + pluginAPI *plugin.API } // MutexFactory is used by the store in plugin mode to generate @@ -42,6 +45,7 @@ func New(params Params) (*SQLStore, error) { logger: params.Logger, isPlugin: params.IsPlugin, NewMutexFn: params.NewMutexFn, + pluginAPI: params.PluginAPI, } err := store.Migrate() diff --git a/server/utils/utils.go b/server/utils/utils.go index e0fe3d0af..24329126b 100644 --- a/server/utils/utils.go +++ b/server/utils/utils.go @@ -2,6 +2,7 @@ package utils import ( "encoding/json" + "reflect" "time" mm_model "github.com/mattermost/mattermost-server/v6/model" @@ -54,3 +55,43 @@ func StructToMap(v interface{}) (m map[string]interface{}) { _ = json.Unmarshal(b, &m) return } + +func intersection(a []interface{}, b []interface{}) []interface{} { + set := make([]interface{}, 0) + hash := make(map[interface{}]bool) + av := reflect.ValueOf(a) + bv := reflect.ValueOf(b) + + for i := 0; i < av.Len(); i++ { + el := av.Index(i).Interface() + hash[el] = true + } + + for i := 0; i < bv.Len(); i++ { + el := bv.Index(i).Interface() + if _, found := hash[el]; found { + set = append(set, el) + } + } + + return set +} + +func Intersection(x ...[]interface{}) []interface{} { + if len(x) == 0 { + return nil + } + + if len(x) == 1 { + return x[0] + } + + result := x[0] + i := 1 + for i < len(x) { + result = intersection(result, x[i]) + i++ + } + + return result +}