🐛(frontend) warn before uploading a file over the size limit - #2577
🐛(frontend) warn before uploading a file over the size limit#2577RISK-alt wants to merge 1 commit into
Conversation
0b81d09 to
5e4e995
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughThe config endpoint now exposes Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Duplicate of #2522 |
Dropping a file larger than the allowed size showed a bare "unknown error" in the editor. The proxy in front of the API cuts the request and answers a 413 with an HTML body, so errorCauses threw while parsing it as JSON and no cause ever reached the error panel. The size limit the backend already enforces is now exposed by the config endpoint, and the editor checks the file against it before sending anything, with the same toast wording the document import uses. errorCauses no longer throws on a body it cannot parse, and a 413 without a usable cause falls back to an explicit message, which covers the instances whose proxy limit is lower than the application one. The size formatting duplicated in the import hook moved to a shared util. Signed-off-by: risk-alt <aldu6974@gmail.com>
5e4e995 to
1591f94
Compare
Purpose
Fixes #2460
Dropping a file over the limit into the editor shows the bare "unknown error" from the issue's screenshot. Two things are missing.
Nothing checks the size before sending. The backend does enforce
DOCUMENT_IMAGE_MAX_SIZEinFileUploadSerializer.validate_file, but the setting is not part of/api/v1.0/config/, so the frontend has nothing to check against. The user waits for the whole upload before learning it was refused — and with nginx in front, the request never even reaches Django.And the 413 carries no readable cause.
errorCausesparses the body unconditionally:nginx answers its 413 with an HTML error page, so
response.json()rejects,errorCausesthrows instead of returning, andAPIErroris never built. The editor is left with an error carrying no cause, which is what it renders as "unknown error".Proposal
DOCUMENT_IMAGE_MAX_SIZEin the config endpoint, next toCONVERSION_FILE_MAX_SIZEwhich the import feature already uses that wayuseUploadFilebefore calling the API, and show an error toast naming the file and the limit — same wording as the document import, so both paths read alikeerrorCausestolerate a body it cannot parse rather than throwing, which fixes every proxy-level error, not just this oneDOCUMENT_IMAGE_MAX_SIZEstill get something readableThe size-formatting logic was inlined in
useImport; it moved toformatFileSizeinutils/string.tsand is now shared by both call sites rather than copied.This follows what
🛂(frontend) use max size and extension from config(b8bdcbf) did for the import feature: same shape, same set of files.I did not touch
translations.json— the English source strings are picked up by the Crowdin sync.Testing
Added:
formatFileSizeunit testserrorCausestest for a non-JSON bodyuseUploadFiletests: a file under the limit reaches the API, a file over it is rejected with the toast and no request at allFull impress suite on Node 26: 284 passing. Four failures in
features/auth/__tests__/utils.test.tsxanddocs-grid/components/__tests__/DocsGridItemDate.test.tsxare pre-existing — they fail identically on a cleanmainhere,localStoragenot being available under my local Node.tsc --noEmitandeslintare clean.Note for reviewers:
CONFIGinapps/e2e/__tests__/app-impress/utils-common.tshad to gain the new key, sinceconfig.spec.tsassertstoStrictEqual(CONFIG)against the live payload.External contributions
General requirements
CI requirements
git commit --signoff(DCO compliance)git commit -S)<gitmoji>(type) title description## [Unreleased]section (if noticeable change)AI requirements