GH-2890 Copy files when duplicating boards (#2915)

* copy files when duplicating boards

* fix unit tests

* Update server/app/boards.go

Co-authored-by: Miguel de la Cruz <mgdelacroix@gmail.com>

* Update server/app/boards.go

Co-authored-by: Miguel de la Cruz <mgdelacroix@gmail.com>

* fix review commit

Co-authored-by: Miguel de la Cruz <mgdelacroix@gmail.com>
Co-authored-by: Mattermod <mattermod@users.noreply.github.com>

server/app/blocks.go

@@ -186,49 +186,63 @@ func (a *App) InsertBlocks(blocks []model.Block, modifiedByID string, allowNotif
 	return blocks, nil
 }
 
-func (a *App) CopyCardFiles(sourceBoardID string, blocks []model.Block) error {
+func (a *App) CopyCardFiles(sourceBoardID string, copiedBlocks []model.Block) error {
 	// Images attached in cards have a path comprising the card's board ID.
 	// When we create a template from this board, we need to copy the files
 	// with the new board ID in path.
 	// Not doing so causing images in templates (and boards created from this
 	// template) to fail to load.
 
-	// look up ID of source board, which may be different than the blocks.
+	// look up ID of source sourceBoard, which may be different than the blocks.
-	board, err := a.GetBlockByID(sourceBoardID)
+	sourceBoard, err := a.GetBoard(sourceBoardID)
-	if err != nil || board == nil {
+	if err != nil || sourceBoard == nil {
-		return fmt.Errorf("cannot fetch board %s for CopyCardFiles: %w", sourceBoardID, err)
+		return fmt.Errorf("cannot fetch source board %s for CopyCardFiles: %w", sourceBoardID, err)
 	}
 
-	for i := range blocks {
+	var destTeamID string
-		block := blocks[i]
+	var destBoardID string
+
+	for i := range copiedBlocks {
+		block := copiedBlocks[i]
 
 		fileName, ok := block.Fields["fileId"]
-		if block.Type == model.TypeImage && ok {
+		if !ok || fileName == "" {
-			// create unique filename in case we are copying cards within the same board.
+			continue // doesn't have a file attachment
-			ext := filepath.Ext(fileName.(string))
+		}
-			destFilename := utils.NewID(utils.IDTypeNone) + ext
 
-			sourceFilePath := filepath.Join(sourceBoardID, fileName.(string))
+		// create unique filename in case we are copying cards within the same board.
-			destinationFilePath := filepath.Join(block.BoardID, destFilename)
+		ext := filepath.Ext(fileName.(string))
+		destFilename := utils.NewID(utils.IDTypeNone) + ext
 
-			a.logger.Debug(
+		if destBoardID == "" || block.BoardID != destBoardID {
-				"Copying card file",
+			destBoardID = block.BoardID
+			destBoard, err := a.GetBoard(destBoardID)
+			if err != nil {
+				return fmt.Errorf("cannot fetch destination board %s for CopyCardFiles: %w", sourceBoardID, err)
+			}
+			destTeamID = destBoard.TeamID
+		}
+
+		sourceFilePath := filepath.Join(sourceBoard.TeamID, sourceBoard.ID, fileName.(string))
+		destinationFilePath := filepath.Join(destTeamID, block.BoardID, destFilename)
+
+		a.logger.Debug(
+			"Copying card file",
+			mlog.String("sourceFilePath", sourceFilePath),
+			mlog.String("destinationFilePath", destinationFilePath),
+		)
+
+		if err := a.filesBackend.CopyFile(sourceFilePath, destinationFilePath); err != nil {
+			a.logger.Error(
+				"CopyCardFiles failed to copy file",
 				mlog.String("sourceFilePath", sourceFilePath),
 				mlog.String("destinationFilePath", destinationFilePath),
+				mlog.Err(err),
 			)
 
-			if err := a.filesBackend.CopyFile(sourceFilePath, destinationFilePath); err != nil {
+			return err
-				a.logger.Error(
-					"CopyCardFiles failed to copy file",
-					mlog.String("sourceFilePath", sourceFilePath),
-					mlog.String("destinationFilePath", destinationFilePath),
-					mlog.Err(err),
-				)
-
-				return err
-			}
-			block.Fields["fileId"] = destFilename
 		}
+		block.Fields["fileId"] = destFilename
 	}
 
 	return nil

server/app/boards.go

@@ -7,6 +7,8 @@ import (
 	"github.com/mattermost/focalboard/server/model"
 	"github.com/mattermost/focalboard/server/services/notify"
 	"github.com/mattermost/focalboard/server/utils"
+
+	"github.com/mattermost/mattermost-server/v6/shared/mlog"
 )
 
 var (
@@ -145,6 +147,46 @@ func (a *App) DuplicateBoard(boardID, userID, toTeam string, asTemplate bool) (*
 	if err != nil {
 		return nil, nil, err
 	}
+
+	// copy any file attachments from the duplicated blocks.
+	if err = a.CopyCardFiles(boardID, bab.Blocks); err != nil {
+		dbab := model.NewDeleteBoardsAndBlocksFromBabs(bab)
+		if err = a.store.DeleteBoardsAndBlocks(dbab, userID); err != nil {
+			a.logger.Error("Cannot delete board after duplication error when copying block's files", mlog.String("boardID", bab.Boards[0].ID), mlog.Err(err))
+		}
+		return nil, nil, fmt.Errorf("could not copy files while duplicating board %s: %w", boardID, err)
+	}
+
+	// bab.Blocks now has updated file ids for any blocks containing files.  We need to store them.
+	blockIDs := make([]string, 0)
+	blockPatches := make([]model.BlockPatch, 0)
+
+	for _, block := range bab.Blocks {
+		if fileID, ok := block.Fields["fileId"]; ok {
+			blockIDs = append(blockIDs, block.ID)
+			blockPatches = append(blockPatches, model.BlockPatch{
+				UpdatedFields: map[string]interface{}{
+					"fileId": fileID,
+				},
+			})
+		}
+	}
+	a.logger.Debug("Duplicate boards patching file IDs", mlog.Int("count", len(blockIDs)))
+
+	if len(blockIDs) != 0 {
+		patches := &model.BlockPatchBatch{
+			BlockIDs:     blockIDs,
+			BlockPatches: blockPatches,
+		}
+		if err = a.store.PatchBlocks(patches, userID); err != nil {
+			dbab := model.NewDeleteBoardsAndBlocksFromBabs(bab)
+			if err = a.store.DeleteBoardsAndBlocks(dbab, userID); err != nil {
+				a.logger.Error("Cannot delete board after duplication error when updating block's file info", mlog.String("boardID", bab.Boards[0].ID), mlog.Err(err))
+			}
+			return nil, nil, fmt.Errorf("could not patch file IDs while duplicating board %s: %w", boardID, err)
+		}
+	}
+
 	a.blockChangeNotifier.Enqueue(func() error {
 		teamID := ""
 		for _, board := range bab.Boards {

server/app/onboarding_test.go

@@ -29,6 +29,7 @@ func TestPrepareOnboardingTour(t *testing.T) {
 		th.Store.EXPECT().DuplicateBoard(welcomeBoard.ID, userID, teamID, false).Return(&model.BoardsAndBlocks{Boards: []*model.Board{&welcomeBoard}},
 			nil, nil)
 		th.Store.EXPECT().GetMembersForBoard(welcomeBoard.ID).Return([]*model.BoardMember{}, nil).Times(2)
+		th.Store.EXPECT().GetBoard(welcomeBoard.ID).Return(&welcomeBoard, nil).AnyTimes()
 
 		privateWelcomeBoard := model.Board{
 			ID:         "board_id_1",
@@ -74,6 +75,8 @@ func TestCreateWelcomeBoard(t *testing.T) {
 		th.Store.EXPECT().DuplicateBoard(welcomeBoard.ID, userID, teamID, false).
 			Return(&model.BoardsAndBlocks{Boards: []*model.Board{&welcomeBoard}}, nil, nil)
 		th.Store.EXPECT().GetMembersForBoard(welcomeBoard.ID).Return([]*model.BoardMember{}, nil).Times(2)
+		th.Store.EXPECT().GetBoard(welcomeBoard.ID).Return(&welcomeBoard, nil).AnyTimes()
+
 		privateWelcomeBoard := model.Board{
 			ID:         "board_id_1",
 			Title:      "Welcome to Boards!",

server/model/boards_and_blocks.go

@@ -73,6 +73,22 @@ type DeleteBoardsAndBlocks struct {
 	Blocks []string `json:"blocks"`
 }
 
+func NewDeleteBoardsAndBlocksFromBabs(babs *BoardsAndBlocks) *DeleteBoardsAndBlocks {
+	boardIDs := make([]string, 0, len(babs.Boards))
+	blockIDs := make([]string, 0, len(babs.Boards))
+
+	for _, board := range babs.Boards {
+		boardIDs = append(boardIDs, board.ID)
+	}
+	for _, block := range babs.Blocks {
+		blockIDs = append(blockIDs, block.ID)
+	}
+	return &DeleteBoardsAndBlocks{
+		Boards: boardIDs,
+		Blocks: blockIDs,
+	}
+}
+
 func (dbab *DeleteBoardsAndBlocks) IsValid() error {
 	if len(dbab.Boards) == 0 {
 		return ErrNoBoardsInBoardsAndBlocks