fix: add error messages for max files uploaded, update uicore #843
Merged
fix: add error messages for max files uploaded, update uicore #843
Conversation
📝 WalkthroughWalkthroughThe upload component was upgraded from UploadInputV2 to UploadInputV3, adding i18n support for maximum file limit errors. The component now computes the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/mui/formik-inputs/mui-formik-upload.js (1)
59-62:⚠️ Potential issue | 🟠 MajorRemoval matching uses the wrong filename field and can fail to remove uploaded items.
Line 61 checks
i.filename, but newly uploaded entries are stored withfile_name(Line 41). This can leave files in Formik state after removal and incorrectly keep max-files state locked.💡 Proposed fix
const handleRemove = (imageFile) => { - const updated = (field.value || []).filter( - (i) => i.filename !== imageFile.name - ); + const targetId = imageFile?.id; + const targetName = + imageFile?.name ?? imageFile?.filename ?? imageFile?.file_name; + const updated = (field.value || []).filter((i) => { + if (targetId !== undefined && i.id !== undefined) return i.id !== targetId; + const currentName = i.file_name ?? i.filename ?? i.file_path ?? i.file_url; + return currentName !== targetName; + }); helpers.setValue(updated); if (onDelete) { onDelete(imageFile.id); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/mui/formik-inputs/mui-formik-upload.js` around lines 59 - 62, The removal predicate in handleRemove is comparing uploaded entries using the wrong key; change the filter to compare the Formik entry's file_name field to the incoming imageFile.name (i.e. replace i.filename with i.file_name) so uploaded items are correctly removed from field.value and max-files state unlocks; update any related comparisons in the component that assume filename to instead use file_name (or normalize to a single key) to ensure consistency.
🧹 Nitpick comments (1)
src/components/mui/formik-inputs/mui-formik-upload.js (1)
86-87: ComputecanAddMore()once and reuse it for both props.This keeps
canAddanderrorperfectly aligned and avoids duplicate evaluation.♻️ Suggested cleanup
+ const canAdd = canAddMore(); + return ( <> @@ djsConfig={{ withCredentials: true }} maxFiles={maxFiles} - canAdd={canAddMore()} - error={!canAddMore() ? T.translate("errors.maximum_files") : undefined} + canAdd={canAdd} + error={!canAdd ? T.translate("errors.maximum_files") : undefined} parallelChunkUploads /> </> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/mui/formik-inputs/mui-formik-upload.js` around lines 86 - 87, Compute canAddMore() once into a local constant (e.g., const canAdd = canAddMore()) inside the MuiFormikUpload component and then pass that constant to the canAdd prop and use it to derive error (error={!canAdd ? T.translate("errors.maximum_files") : undefined}); update references to use that constant so both props are consistent and the function is not evaluated twice.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/components/mui/formik-inputs/mui-formik-upload.js`:
- Around line 59-62: The removal predicate in handleRemove is comparing uploaded
entries using the wrong key; change the filter to compare the Formik entry's
file_name field to the incoming imageFile.name (i.e. replace i.filename with
i.file_name) so uploaded items are correctly removed from field.value and
max-files state unlocks; update any related comparisons in the component that
assume filename to instead use file_name (or normalize to a single key) to
ensure consistency.
---
Nitpick comments:
In `@src/components/mui/formik-inputs/mui-formik-upload.js`:
- Around line 86-87: Compute canAddMore() once into a local constant (e.g.,
const canAdd = canAddMore()) inside the MuiFormikUpload component and then pass
that constant to the canAdd prop and use it to derive error (error={!canAdd ?
T.translate("errors.maximum_files") : undefined}); update references to use that
constant so both props are consistent and the function is not evaluated twice.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c49f5cbd-5350-4dea-a691-8e3619c28050
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
package.jsonsrc/components/mui/formik-inputs/mui-formik-upload.jssrc/i18n/en.json
… UploadInputV3 Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
tomrndom
force-pushed
the
fix/pages-upload-input-v3-error
branch
from
5cb37fa to
7d082db
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/mui/formik-inputs/mui-formik-upload.js (1)
86-87: ComputecanAddonce to avoid duplicated evaluation.This is minor, but deriving the boolean once improves readability and avoids repeating logic in adjacent props.
♻️ Suggested refactor
+ {(() => { + const canAdd = canAddMore(); + return ( <UploadInputV3 id={id} name={name} onUploadComplete={handleUploadComplete} value={getInputValue()} mediaType={mediaType} onRemove={handleRemove} postUrl={`${window.FILE_UPLOAD_API_BASE_URL}/api/v1/files/upload`} djsConfig={{ withCredentials: true }} maxFiles={maxFiles} - canAdd={canAddMore()} - error={!canAddMore() ? T.translate("errors.maximum_files") : undefined} + canAdd={canAdd} + error={!canAdd ? T.translate("errors.maximum_files") : undefined} parallelChunkUploads /> + ); + })()}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/mui/formik-inputs/mui-formik-upload.js` around lines 86 - 87, Compute the can-add boolean once and reuse it instead of calling canAddMore() twice: call canAddMore() once (e.g., const canAdd = canAddMore()) near the component render and replace the two uses with canAdd for the canAdd prop and for the error conditional (error={!canAdd ? T.translate("errors.maximum_files") : undefined}); update references in mui-formik-upload related to canAddMore(), keeping function name canAddMore for clarity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/mui/formik-inputs/mui-formik-upload.js`:
- Around line 86-87: Compute the can-add boolean once and reuse it instead of
calling canAddMore() twice: call canAddMore() once (e.g., const canAdd =
canAddMore()) near the component render and replace the two uses with canAdd for
the canAdd prop and for the error conditional (error={!canAdd ?
T.translate("errors.maximum_files") : undefined}); update references in
mui-formik-upload related to canAddMore(), keeping function name canAddMore for
clarity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1eb9bf95-befc-4a1a-91b5-4c2b6437b9e5
📒 Files selected for processing (2)
src/components/mui/formik-inputs/mui-formik-upload.jssrc/i18n/en.json
✅ Files skipped from review due to trivial changes (1)
- src/i18n/en.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ref: https://app.clickup.com/t/86b90ngzy
Signed-off-by: Tomás Castillo tcastilloboireau@gmail.com
Summary by CodeRabbit
Release Notes