-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MM-61043 Error deleting board if fileID is empty #23
Conversation
@Rajat-Dabade can you add the steps to reproduce this bug in the ticket? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good with one caveat
server/app/files.go
Outdated
fileInfoID := getFileInfoID(parts[0]) | ||
|
||
if (fileInfoID) == "" { | ||
return nil, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this case also return an error? I'd expect that this sort of method would always return either a FileInfo or an error, but this case returns nil for both of them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, returning an error with message file not found
.
Also, excellent explanation of your investigation here. I'm surprised that we keep the file extension on the uploaded file and that we have to calculate the file ID from the filename, so it's good to know that the code works that way |
Summary
https://hub.mattermost.com/icu/pl/qqb9tqrzdtfajyfrysd39ecaro
Ticket Link
https://mattermost.atlassian.net/browse/MM-61043