Skip to content

Commit

Permalink
Fix cards not deleting properly. (#4746) (#4753)
Browse files Browse the repository at this point in the history
* Fix cards not deleting properly.

* Review feedback

* Test and lint fixes.

* Fix tests.

(cherry picked from commit c3b1c82)

Co-authored-by: Christopher Speller <[email protected]>
  • Loading branch information
sbishel and crspeller authored May 31, 2023
1 parent 304afa9 commit 39c8253
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 15 deletions.
3 changes: 2 additions & 1 deletion mattermost-plugin/server/manifest.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 0 additions & 14 deletions server/app/blocks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
70 changes: 70 additions & 0 deletions server/services/store/sqlstore/blocks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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})
Expand Down Expand Up @@ -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})
Expand Down

0 comments on commit 39c8253

Please sign in to comment.