Fix cards not deleting properly. (#4746) (#4755)

* Fix cards not deleting properly. (#4746)

* Fix cards not deleting properly.

* Review feedback

* Test and lint fixes.

* Fix tests.

(cherry picked from commit c3b1c82b1a)

* fix bad merge

---------

Co-authored-by: Christopher Speller <crspeller@gmail.com>
Co-authored-by: Mattermost Build <build@mattermost.com>

server/app/blocks.go

@@ -410,20 +410,6 @@ func (a *App) DeleteBlockAndNotify(blockID string, modifiedBy string, disableNot
 		return err
 	}
 
-	if block.Type == model.TypeImage {
-		fileName, fileIDExists := block.Fields["fileId"]
-		if fileName, fileIDIsString := fileName.(string); fileIDExists && fileIDIsString {
-			filePath := filepath.Join(block.BoardID, fileName)
-			err = a.filesBackend.RemoveFile(filePath)
-
-			if err != nil {
-				a.logger.Error("Error deleting image file",
-					mlog.String("FilePath", filePath),
-					mlog.Err(err))
-			}
-		}
-	}
-
 	a.blockChangeNotifier.Enqueue(func() error {
 		a.wsAdapter.BroadcastBlockDelete(board.TeamID, blockID, block.BoardID)
 		a.metrics.IncrementBlocksDeleted(1)

server/services/store/sqlstore/blocks.go

@@ -365,6 +365,14 @@ func (s *SQLStore) deleteBlock(db sq.BaseRunner, blockID string, modifiedBy stri
 	return s.deleteBlockAndChildren(db, blockID, modifiedBy, false)
 }
 
+func retrieveFileIDFromBlockFieldStorage(id string) string {
+	parts := strings.Split(id, ".")
+	if len(parts) < 1 {
+		return ""
+	}
+	return parts[0][1:]
+}
+
 func (s *SQLStore) deleteBlockAndChildren(db sq.BaseRunner, blockID string, modifiedBy string, keepChildren bool) error {
 	block, err := s.getBlock(db, blockID)
 	if model.IsErrNotFound(err) {
@@ -415,6 +423,30 @@ func (s *SQLStore) deleteBlockAndChildren(db sq.BaseRunner, blockID string, modi
 		return err
 	}
 
+	// fileId and attachmentId shoudn't exist at the same time
+	fileID := ""
+	fileIDWithExtention, fileIDExists := block.Fields["fileId"]
+	if fileIDExists {
+		fileID = retrieveFileIDFromBlockFieldStorage(fileIDWithExtention.(string))
+	}
+
+	if fileID == "" {
+		attachmentIDWithExtention, attachmentIDExists := block.Fields["attachmentId"]
+		if attachmentIDExists {
+			fileID = retrieveFileIDFromBlockFieldStorage(attachmentIDWithExtention.(string))
+		}
+	}
+
+	if fileID != "" {
+		deleteFileInfoQuery := s.getQueryBuilder(db).
+			Update("FileInfo").
+			Set("DeleteAt", model.GetMillis()).
+			Where(sq.Eq{"id": fileID})
+		if _, err := deleteFileInfoQuery.Exec(); err != nil {
+			return err
+		}
+	}
+
 	deleteQuery := s.getQueryBuilder(db).
 		Delete(s.tablePrefix + "blocks").
 		Where(sq.Eq{"id": blockID})
@@ -931,6 +963,44 @@ func (s *SQLStore) deleteBlockChildren(db sq.BaseRunner, boardID string, parentI
 		return err
 	}
 
+	fileDeleteQuery := s.getQueryBuilder(db).
+		Select(s.blockFields("")...).
+		From(s.tablePrefix + "blocks").
+		Where(sq.Eq{"board_id": boardID})
+
+	rows, err := fileDeleteQuery.Query()
+	if err != nil {
+		return err
+	}
+	defer s.CloseRows(rows)
+	blocks, err := s.blocksFromRows(rows)
+	if err != nil {
+		return err
+	}
+
+	fileIDs := make([]string, 0, len(blocks))
+	for _, block := range blocks {
+		fileIDWithExtention, fileIDExists := block.Fields["fileId"]
+		if fileIDExists {
+			fileIDs = append(fileIDs, retrieveFileIDFromBlockFieldStorage(fileIDWithExtention.(string)))
+		}
+		attachmentIDWithExtention, attachmentIDExists := block.Fields["attachmentId"]
+		if attachmentIDExists {
+			fileIDs = append(fileIDs, retrieveFileIDFromBlockFieldStorage(attachmentIDWithExtention.(string)))
+		}
+	}
+
+	if len(fileIDs) > 0 {
+		deleteFileInfoQuery := s.getQueryBuilder(db).
+			Update("FileInfo").
+			Set("DeleteAt", model.GetMillis()).
+			Where(sq.Eq{"id": fileIDs})
+
+		if _, err := deleteFileInfoQuery.Exec(); err != nil {
+			return err
+		}
+	}
+
 	deleteQuery := s.getQueryBuilder(db).
 		Delete(s.tablePrefix + "blocks").
 		Where(sq.Eq{"board_id": boardID})