Small improve on deleting attachements #3145
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3145 +/- ##
========================================
- Coverage 34.9% 34.7% -0.2%
========================================
Files 277 277
Lines 40108 40116 +8
========================================
- Hits 13998 13922 -76
- Misses 24061 24159 +98
+ Partials 2049 2035 -14
Continue to review full report at Codecov.
|
|
LGTM |
models/attachment.go
Outdated
| return 0, nil | ||
| } | ||
|
|
||
| var ids = make([]int64, 0, len(attachments)) | ||
| for i, a := range attachments { | ||
| if remove { | ||
| if err := os.Remove(a.LocalPath()); err != nil { | ||
| return i, err |
There was a problem hiding this comment.
In this case, there are some attachments which we have already deleted from the filesystem. Should we also remove them from the DB, so that the DB doesn't contain non-existent attachments?
There was a problem hiding this comment.
additional check could be added to check if error is about nonexistent file but if error is that file can not be deleted (access denied etc) than it should stay as it is now
ae1f337
to
2942b7a
Compare
|
@lafriks @ethantkoenig done. |
|
@ethantkoenig the tests have existed in |
|
LGTM |
As title.