-
-
Notifications
You must be signed in to change notification settings - Fork 835
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
Add extra error handling for avatar file size & large payload #3042
Conversation
Should we add a translation key for the attribute? The screenshot shows |
|
Any thoughts on
To clarify, that'd change it to show required message when no file is provided, and upload failure message on the other errors, which would be clearer as to the issue. |
I would be in favor. |
…d of 'no file' message
EDIT: Just saw you made an issue (#3044). 😂 After further testing, not providing an avatar seems to result in 500 because there's Not sure if that should be addressed in this PR - to keep the code clean we'd need to change signature of |
Let's deal with that separately https://github.com/flarum/core/issues/3044, essentially we'd just want to introduce a new |
Anything missing with this PR? Looking back at it the locale key should probably be changed from |
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.
I don't remember, it looks good though, let's just change that key and merge.
Fixes #2888
Changes proposed in this pull request:
UPLOAD_ERR_INI_SIZE
&UPLOAD_ERR_FORM_SIZE
, andUPLOAD_ERR_PARTIAL
Reviewers should focus on:
UPLOAD_ERR_NO_FILE
? That'd make more sense as only NO_FILE is a required issue, and the others are internal so we can say failed.Screenshot
Confirmed
composer test
).