Add extra error handling for avatar file size & large payload

[dsevillamartin]

Fixes #2888

Changes proposed in this pull request:

• Add separate error messages during upload for UPLOAD_ERR_INI_SIZE & UPLOAD_ERR_FORM_SIZE, and UPLOAD_ERR_PARTIAL
• Add frontend handling of 413 status code (Payload Too Large)

Reviewers should focus on:

• Do we perhaps want to say "failed to upload" for the other errors and just "required" for 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.
• This file logic, validation and/or error handling should probably be extracted so that other areas & extensions can use it... out of scope for this however.

Screenshot

Confirmed

• Frontend changes: tested on a local Flarum installation.
• Backend changes: tests are green (run composer test).

[askvortsov1]

Should we add a translation key for the attribute? The screenshot shows :attribute which doesn't seem right.

[SychO9]

Should we add a translation key for the attribute? The screenshot shows :attribute which doesn't seem right.

#2933

[dsevillamartin]

Any thoughts on

Do we perhaps want to say "failed to upload" for the other errors and just "required" for 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.

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.

[SychO9]

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.

[dsevillamartin]

EDIT: Just saw you made an issue (#3044). 😂

After further testing, not providing an avatar seems to result in 500 because there's $attributes['avatar'] is not an UploadedFileInterface... not sure exactly when the error would be UPLOAD_ERR_NO_FILE 🤷‍♂️

Not sure if that should be addressed in this PR - to keep the code clean we'd need to change signature of assertFileRequired, which would be potentially breaking (not sure if we'd classify that as breaking tbh) to allow null

[SychO9]

After further testing, not providing an avatar seems to result in 500 because there's $attributes['avatar'] is not an UploadedFileInterface... not sure exactly when the error would be UPLOAD_ERR_NO_FILE

Not sure if that should be addressed in this PR - to keep the code clean we'd need to change signature of assertFileRequired, which would be potentially breaking (not sure if we'd classify that as breaking tbh) to allow null

Let's deal with that separately https://github.com/flarum/core/issues/3044, essentially we'd just want to introduce a new assertFile method that accepts any type as parameter and checks that the parameter is an instance of the interface. Which shouldn't be backwards incompatible.

[dsevillamartin]

Anything missing with this PR? Looking back at it the locale key should probably be changed from file_uploaded to file_upload_failed... the former doesn't make sense given the context.

[SychO9]

I don't remember, it looks good though, let's just change that key and merge.