Fixed duplicate attachment in board template (#4444)

* Fixed duplicate attachment in board template

* Linter fixes

* Duplicate attachment fix

* Code optimismed

* Linter fixes

* Updated test cases

* update some error handling, update attachments on duplicate card

* Fixed attachment section width

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
Co-authored-by: Scott Bishel <scott.bishel@mattermost.com>
Co-authored-by: Harshil Sharma <harshilsharma63@gmail.com>

server/app/blocks.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"path/filepath"
+	"strings"
 
 	"github.com/mattermost/focalboard/server/model"
 	"github.com/mattermost/focalboard/server/services/notify"
@@ -309,14 +310,26 @@ func (a *App) CopyCardFiles(sourceBoardID string, copiedBlocks []*model.Block) e
 
 	for i := range copiedBlocks {
 		block := copiedBlocks[i]
+		fileName := ""
+		isOk := false
 
-		fileName, ok := block.Fields["fileId"]
-		if !ok || fileName == "" {
-			continue // doesn't have a file attachment
+		switch block.Type {
+		case model.TypeImage:
+			fileName, isOk = block.Fields["fileId"].(string)
+			if !isOk || fileName == "" {
+				continue
+			}
+		case model.TypeAttachment:
+			fileName, isOk = block.Fields["attachmentId"].(string)
+			if !isOk || fileName == "" {
+				continue
+			}
+		default:
+			continue
 		}
 
 		// create unique filename in case we are copying cards within the same board.
-		ext := filepath.Ext(fileName.(string))
+		ext := filepath.Ext(fileName)
 		destFilename := utils.NewID(utils.IDTypeNone) + ext
 
 		if destBoardID == "" || block.BoardID != destBoardID {
@@ -328,7 +341,7 @@ func (a *App) CopyCardFiles(sourceBoardID string, copiedBlocks []*model.Block) e
 			destTeamID = destBoard.TeamID
 		}
 
-		sourceFilePath := filepath.Join(sourceBoard.TeamID, sourceBoard.ID, fileName.(string))
+		sourceFilePath := filepath.Join(sourceBoard.TeamID, sourceBoard.ID, fileName)
 		destinationFilePath := filepath.Join(destTeamID, block.BoardID, destFilename)
 
 		a.logger.Debug(
@@ -345,7 +358,24 @@ func (a *App) CopyCardFiles(sourceBoardID string, copiedBlocks []*model.Block) e
 				mlog.Err(err),
 			)
 		}
-		block.Fields["fileId"] = destFilename
+		if block.Type == model.TypeAttachment {
+			block.Fields["attachmentId"] = destFilename
+			parts := strings.Split(fileName, ".")
+			fileInfoID := parts[0][1:]
+			fileInfo, err := a.store.GetFileInfo(fileInfoID)
+			if err != nil {
+				return fmt.Errorf("CopyCardFiles: cannot retrieve original fileinfo: %w", err)
+			}
+			newParts := strings.Split(destFilename, ".")
+			newFileID := newParts[0][1:]
+			fileInfo.Id = newFileID
+			err = a.store.SaveFileInfo(fileInfo)
+			if err != nil {
+				return fmt.Errorf("CopyCardFiles: cannot create fileinfo: %w", err)
+			}
+		} else {
+			block.Fields["fileId"] = destFilename
+		}
 	}
 
 	return nil

server/app/boards.go

@@ -202,13 +202,21 @@ func (a *App) DuplicateBoard(boardID, userID, toTeam string, asTemplate bool) (*
 	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,
-				},
-			})
+		fieldName := ""
+		if block.Type == model.TypeImage {
+			fieldName = "fileId"
+		} else if block.Type == model.TypeAttachment {
+			fieldName = "attachmentId"
+		}
+		if fieldName != "" {
+			if fieldID, ok := block.Fields[fieldName]; ok {
+				blockIDs = append(blockIDs, block.ID)
+				blockPatches = append(blockPatches, model.BlockPatch{
+					UpdatedFields: map[string]interface{}{
+						fieldName: fieldID,
+					},
+				})
+			}
 		}
 	}
 	a.logger.Debug("Duplicate boards patching file IDs", mlog.Int("count", len(blockIDs)))

webapp/cypress/integration/cardURLProperty.ts

@@ -98,13 +98,9 @@ describe('Card URL Property', () => {
     const addView = (type: ViewType) => {
         cy.log(`**Add ${type} view**`)
 
-        // Intercept and wait for getUser request because it is the last one in the effects for BoardPage
-        // After this last request the BoardPage component will not have additional rerenders
-        cy.intercept('POST', '/api/v2/users').as('getUser')
         cy.findByRole('button', {name: 'View menu'}).click()
         cy.findByText('Add view').realHover()
         cy.findByRole('button', {name: type}).click()
-        cy.wait('@getUser')
         cy.findByRole('textbox', {name: `${type} view`}).should('exist')
     }
 

webapp/src/components/cardDetail/attachment.scss

@@ -1,5 +1,6 @@
 .Attachment {
     display: block;
+    width: 100%;
 
     .attachment-header {
         display: flex;
@@ -13,7 +14,6 @@
         padding-bottom: 20px;
         display: flex;
         overflow-x: auto;
-        width: 550px;
     }
 
     .attachment-plus-icon {

webapp/src/mutator.ts

@@ -12,6 +12,7 @@ import {BoardView, ISortOption, createBoardView, KanbanCalculationFields} from '
 import {Card, createCard} from './blocks/card'
 import {ContentBlock} from './blocks/contentBlock'
 import {CommentBlock} from './blocks/commentBlock'
+import {AttachmentBlock} from './blocks/attachmentBlock'
 import {FilterGroup} from './blocks/filterGroup'
 import octoClient from './octoClient'
 import undoManager from './undomanager'
@@ -26,6 +27,7 @@ import store from './store'
 import {updateBoards} from './store/boards'
 import {updateViews} from './store/views'
 import {updateCards} from './store/cards'
+import {updateAttachments} from './store/attachments'
 import {updateComments} from './store/comments'
 import {updateContents} from './store/contents'
 import {addBoardUsers, removeBoardUsersById} from './store/users'
@@ -35,6 +37,7 @@ function updateAllBoardsAndBlocks(boards: Board[], blocks: Block[]) {
         store.dispatch(updateBoards(boards.filter((b: Board) => b.deleteAt !== 0) as Board[]))
         store.dispatch(updateViews(blocks.filter((b: Block) => b.type === 'view' || b.deleteAt !== 0) as BoardView[]))
         store.dispatch(updateCards(blocks.filter((b: Block) => b.type === 'card' || b.deleteAt !== 0) as Card[]))
+        store.dispatch(updateAttachments(blocks.filter((b: Block) => b.type === 'attachment' || b.deleteAt !== 0) as AttachmentBlock[]))
         store.dispatch(updateComments(blocks.filter((b: Block) => b.type === 'comment' || b.deleteAt !== 0) as CommentBlock[]))
         store.dispatch(updateContents(blocks.filter((b: Block) => b.type !== 'card' && b.type !== 'view' && b.type !== 'board' && b.type !== 'comment') as ContentBlock[]))
     })

webapp/src/pages/boardPage/boardPage.tsx

@@ -215,13 +215,15 @@ const BoardPage = (props: Props): JSX.Element => {
                     UserSettings.setLastViewId(match.params.boardId, viewId)
                 }
             }
-
-            if (!props.readonly && me) {
-                loadOrJoinBoard(me.id, teamId, match.params.boardId)
-            }
         }
     }, [teamId, match.params.boardId, viewId, me?.id])
 
+    useEffect(() => {
+        if (match.params.boardId && !props.readonly && me) {
+            loadOrJoinBoard(me.id, teamId, match.params.boardId)
+        }
+    }, [teamId, match.params.boardId, me?.id])
+
     const handleUnhideBoard = async (boardID: string) => {
         if (!me || !category) {
             return

webapp/src/store/attachments.ts

@@ -26,7 +26,9 @@ const attachmentSlice = createSlice({
                         state.attachmentsByCard[attachment.parentId] = [attachment]
                         return
                     }
-                    state.attachmentsByCard[attachment.parentId].push(attachment)
+                    if (state.attachmentsByCard[attachment.parentId].findIndex((a) => a.id === attachment.id) === -1) {
+                        state.attachmentsByCard[attachment.parentId].push(attachment)
+                    }
                 } else {
                     const parentId = state.attachments[attachment.id]?.parentId
                     if (!state.attachmentsByCard[parentId]) {