KPMP 6797 create file drop box - #78
Conversation
WalkthroughThe PR integrates Fine Uploader for YAML uploads, adds Redux upload-status state, and replaces the bulk-upload interface with a three-step workflow. It also adds uploader styling, gallery CSS, new dependencies, and updates the development start script. ChangesUpload workflow
Sequence Diagram(s)sequenceDiagram
participant BulkUpload
participant FileDropzone
participant FineUploader
participant ReduxStore
BulkUpload->>FineUploader: Initialize uploader
FileDropzone->>FineUploader: Select or drop YAML file
FineUploader-->>BulkUpload: Report upload status
BulkUpload->>ReduxStore: Dispatch SET_IS_UPLOADING
ReduxStore-->>FileDropzone: Provide isUploading state
Merge Risk: 🟡 Moderate · up to File-drop uploads currently remain queued instead of starting, upload progress cannot update correctly, and stale callbacks may cause later upload attempts to fail; updated styles also will not be compiled during development. The PR is not merge-ready until these issues are fixed. 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fdd2da42-6b7b-412b-bab9-a7f18d63ea5f
📒 Files selected for processing (12)
package.jsonsrc/actions/Packages/packageActions.jssrc/actions/actionNames.jssrc/components/Upload/BulkUpload.jssrc/components/Upload/BulkUploadContainer.jssrc/components/Upload/FileDropzone.jssrc/components/Upload/UploadReducer.jssrc/components/Upload/fineUploader.jssrc/index.jssrc/index.scsssrc/initialState.jsonsrc/reducers.js
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
| "scripts": { | ||
| "start-js": "react-scripts start", | ||
| "start": "npm-run-all -p watch-css start-js", | ||
| "start": "npm-run-all -p start-js", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep Sass compilation in the development start command.
src/index.js imports src/index.css, but this PR changes src/index.scss. Removing watch-css means npm start does not run build-css, so the new upload styles are not present in the CSS loaded by the app.
Proposed fix
- "start": "npm-run-all -p start-js",
+ "start": "npm-run-all -p start-js watch-css",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "start": "npm-run-all -p start-js", | |
| "start": "npm-run-all -p start-js watch-css", |
| uploader.on('submit', () => { | ||
| let newCount = this.state.filesAdded + 1; | ||
| this.setState( { filesAdded: newCount } ); | ||
| this.isSubmitDisabled(); | ||
| return true; | ||
| }); | ||
|
|
||
| uploader.on('cancel', () => { | ||
| let newCount = this.state.filesAdded - 1; | ||
| this.setState( { filesAdded: newCount }); | ||
| this.isSubmitDisabled(); | ||
| return true; | ||
| }); | ||
|
|
||
| uploader.on('submit', (id, name) => { | ||
| let files = uploader.methods.getUploads({ | ||
| status: [ qq.status.SUBMITTED, qq.status.PAUSED ]}); | ||
|
|
||
| for(let fileIndex in files) { | ||
| let existingName = files[fileIndex].name; | ||
| if (existingName === name) { | ||
| alert("You have already selected " + existingName + " to upload."); | ||
| return false; | ||
| } | ||
| } | ||
| return true; | ||
| }); | ||
|
|
||
| uploader.on('validateBatch', () => { | ||
| if (this.state.submitClicked) { | ||
| return false; | ||
| } | ||
| return true; | ||
| }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
fd -i 'BulkUpload|fineUploader|package.json|yarn.lock|package-lock.json' . || true
printf '%s\n' '--- BulkUpload outline ---'
bulk="$(fd -i -t f 'BulkUpload' . | head -n 1)"
if [ -n "${bulk}" ]; then
ast-grep outline "${bulk}" --view compact || true
printf '%s\n' '--- BulkUpload source ---'
cat -n "${bulk}"
fi
printf '%s\n' '--- FineUploader references ---'
rg -n -S "FineUploader|fineUploader|uploader\.on|uploader\.off|validateBatch|statusChange|submitClicked|componentWillUnmount|handleSubmit" . \
-g '*.js' -g '*.jsx' -g '*.json' -g '*.md' || true
printf '%s\n' '--- dependency manifests ---'
for f in package.json yarn.lock package-lock.json; do
if [ -f "$f" ]; then
printf '%s\n' "--- $f ---"
rg -n -C 3 -S "fine-uploader|fineUploader|fineuploader" "$f" || true
fi
doneRepository: KPMP/libra-web
Length of output: 10710
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- locate source files ---'
git ls-files | rg -i '(^|/)(BulkUpload|fineUploader)(\.[^.]+)?$|package(-lock)?\.json$|yarn\.lock$' || true
printf '%s\n' '--- locate symbols ---'
rg -n -S "class BulkUpload|function BulkUpload|componentWillUnmount|validateBatch|statusChange|submitClicked|uploader\.on|uploader\.off|handleSubmit" src package.json yarn.lock package-lock.json 2>/dev/null || trueRepository: KPMP/libra-web
Length of output: 1706
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- uploader factory ---'
cat -n src/components/Upload/fineUploader.js
printf '%s\n' '--- upload component files ---'
for f in $(git ls-files 'src/components/Upload/*' | sort); do
printf '%s\n' "--- $f ---"
wc -l "$f"
done
printf '%s\n' '--- FileDropzone symbols and source ---'
dropzone="$(fd -i -t f 'FileDropzone' src | head -n 1 || true)"
if [ -n "$dropzone" ]; then
rg -n -C 5 -S "uploader|methods|onSubmit|autoUpload|upload|submit|reset" "$dropzone" || true
cat -n "$dropzone"
fi
printf '%s\n' '--- lockfile package entries ---'
rg -n -C 8 -S '"fine-uploader-wrappers"|"fine-uploader"|"react-fine-uploader"' package-lock.json package.json || true
printf '%s\n' '--- all uploader call sites ---'
rg -n -S "getUploader|FineUploaderTraditional|new FineUploader|\.methods\.(upload|uploadStoredFiles|reset)|\.on\(|\.off\(" src -g '*.js' -g '*.jsx' || trueRepository: KPMP/libra-web
Length of output: 5987
🌐 Web query:
Fine Uploader 5.16.2 validateBatch callback return false off event callback same reference documentation
💡 Result:
In Fine Uploader 5.16.2, the validateBatch callback allows you to perform custom validation on an entire batch of files selected for upload [1][2]. If your validation logic determines that the batch (or any file within it) should be rejected, returning false from this callback will prevent the files from being processed [1][2][3]. Key details regarding the validateBatch callback: 1. Functionality: The callback is triggered whenever files are selected, dropped, or added via addFiles, specifically before the default Fine Uploader validators execute [4][5]. 2. Returning false: If you return false, the entire batch submission is cancelled [2]. If you are using asynchronous validation, you may return a promise instead; calling failure on that promise is equivalent to returning false [4][5][3]. 3. Event reference: When using the jQuery integration, ensure you are using the correct event name validateBatch (case-sensitive) and remember that the first parameter passed to the callback is the event object [2]. Example implementation:.on('validateBatch', function(event, fileOrBlobDataArray) { var totalSize = 0; // Iterate through the array of files/blobs $.each(fileOrBlobDataArray, function(idx, fileOrBlobData) { totalSize += fileOrBlobData.size; }); // Reject the batch if it exceeds a custom limit if (totalSize > maxAllowedBatchSize) { return false; } }); Note that this client-side validation requires File API support (not available in IE9 and older) [2]. As with all client-side checks, this should be paired with server-side validation to ensure security and data integrity [2].
Citations:
- 1: https://docs.fineuploader.com/branch/master/features/validation.html
- 2: https://stackoverflow.com/questions/17698725/file-upload-validation-of-the-whole-batch-of-selected-file
- 3: https://docs.fineuploader.com/branch/master/features/async-tasks-and-promises.html
- 4: https://github.com/FineUploader/fine-uploader/blob/5.16.2/client/typescript/fine-uploader.d.ts
- 5: https://docs.fineuploader.com//api/events.html
🌐 Web query:
github fine-uploader-wrappers on off validateBatch callback source
💡 Result:
In the Fine Uploader Wrappers library, event listeners are managed using the.on method [1][2]. When using this method, the event name can be specified with or without an 'on' prefix [1][2]. If you include the prefix, it must follow strict lower-camel-case formatting (e.g., 'onValidateBatch', not 'onvalidatebatch') [1][2]. The validateBatch callback is a global event that allows you to perform custom validation on an entire batch of selected files before the default Fine Uploader validators execute [3][4][5]. Key details regarding the validateBatch callback: - Purpose: It is commonly used to enforce rules across all files in a single selection, such as limiting the total size of a batch [6]. - Return Value: If the handler returns false, the entire batch will be rejected [3][4][6]. - Asynchronous Support: You may return a Promise for non-blocking validation; the batch will be processed only after the promise is fulfilled [4][5]. - Parameters: The callback typically receives an array of file or blob data (containing name and size) as its primary argument [4][5][6]. If the files were submitted via a tracked button, the button container element is passed as the last argument [4][5][7]. Note: The case-sensitivity of the event name is critical [6]. For the wrappers, using 'validateBatch' or 'onValidateBatch' is required to correctly register the listener in the event pipeline [1][2].
Citations:
- 1: https://github.com/fineuploader/fine-uploader-wrappers
- 2: https://registry.npmjs.org/fine-uploader-wrappers
- 3: https://docs.fineuploader.com/branch/master/features/validation.html
- 4: https://docs.fineuploader.com/tag/4.2.2/api/events.html
- 5: https://docs.fineuploader.com/branch/develop/api/events.html
- 6: https://stackoverflow.com/questions/17698725/file-upload-validation-of-the-whole-batch-of-selected-file
- 7: https://docs.fineuploader.com/tag/4.2.2/features/extra-buttons.html
🌐 Web query:
site:github.com/fineuploader/fine-uploader "off: function" "callback" event handler remove
💡 Result:
In JavaScript and related environments, the mechanism to remove an event handler depends on the specific library or API being used. Standard JavaScript (DOM API) To remove an event listener added via addEventListener, you must use the removeEventListener method. It requires the same event type, the exact same function reference, and the same options (if any) used during registration. element.removeEventListener('click', yourFunction); jQuery In jQuery, the.off method is used to remove event handlers that were previously attached with.on. // Remove all handlers for a specific event $(selector).off('click'); // Remove a specific handler $(selector).off('click', yourFunction); Node.js EventEmitter In Node.js, the EventEmitter class provides both.removeListener and.off methods. The.off method is an alias for.removeListener and functions identically, requiring the event name and the specific callback function reference to be removed. emitter.off('eventName', callbackFunction); // Or emitter.removeListener('eventName', callbackFunction); If you are working with specific libraries like Fine Uploader, event management is typically handled through the library's internal callback configuration rather than generic DOM event removal methods. Always refer to the specific library documentation for its supported API.
Citations:
- 1: https://github.com/fineuploader/fine-uploader
- 2: Fine-uploader seems to catch every drag'n'drop JS events. FineUploader/fine-uploader#1588
- 3: Custom 'cancel' button calls Cancel(id) method, but upload process won't skip to next file FineUploader/fine-uploader#1481
- 4: Possible memory leak when uploading blobs FineUploader/fine-uploader#1711
- 5: How to add params FineUploader/fine-uploader#473
🌐 Web query:
Fine Uploader API off event callback handler reference remove listener
💡 Result:
To remove an event listener in Fine Uploader, you use the off(eventName, handlerFunction) method [1][2]. The off method requires two arguments: 1. eventName: The name of the event (e.g., 'complete', 'error'). You can provide this with or without the 'on' prefix (e.g., 'onComplete' or 'complete') [1][2]. 2. handlerFunction: The exact reference to the function that was originally passed to the on method when the listener was registered [1][2]. Example usage: const completeHandler = (id, name, response) => { // handle completed upload } // Register the listener uploader.on('complete', completeHandler) // Unregister the listener later uploader.off('complete', completeHandler) Important Considerations: - If you are using the jQuery plugin version of Fine Uploader, event handling and removal follow standard jQuery.on and.off patterns [3]. - When using the core library, ensure you are passing the exact function reference; passing an anonymous function or a different function instance will not successfully remove the listener [1][2].
Citations:
- 1: https://github.com/fineuploader/fine-uploader-wrappers
- 2: https://www.npmjs.com/package/fine-uploader-wrappers
- 3: https://docs.fineuploader.com/branch/master/integrating/jquery.html
Remove all four constructor-registered callbacks from the shared uploader on unmount.
Each BulkUpload mount adds two submit handlers, one cancel handler, and one validateBatch handler. componentWillUnmount removes only statusChange. A retained handler from an instance with submitClicked === true can return false and reject file batches on later visits. Store each handler in a stable instance field and remove it with uploader.off using the same function reference.
🧰 Tools
🪛 React Doctor (0.9.3)
[error] 18-18: This class registers a listener or timer during mount without a matching teardown on every unmount path, so it can keep firing after the component unmounts; release it in componentWillUnmount.
Release listeners and timers acquired in componentDidMount/constructor by adding a componentWillUnmount that removes them (or use MobX disposeOnUnmount).
(class-component-missing-component-will-unmount-teardown)
Source: Linters/SAST tools
| handleUploadStatusChange = (id, oldStatus, status) => { | ||
| const removedStatuses = [ | ||
| uploader.qq.status.CANCELED, | ||
| uploader.qq.status.REJECTED, | ||
| ]; | ||
| if (removedStatuses.includes(status)) { | ||
| this.fileIds.delete(id); | ||
| } else { | ||
| this.fileIds.add(id); | ||
| } | ||
|
|
||
| this.setState({hasFiles: this.fileIds.size > 0}); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- candidate files ---'
git ls-files 'src/components/Upload/*' 'src/*' | sed -n '1,120p'
printf '%s\n' '--- BulkUpload outline ---'
ast-grep outline src/components/Upload/BulkUpload.js || true
printf '%s\n' '--- BulkUpload.js ---'
cat -n src/components/Upload/BulkUpload.js
printf '%s\n' '--- related symbols ---'
rg -n -C 4 'setIsUploading|isUploading|statusChange|onStatusChange|addEventListener|validateBatch|submitClicked|handleUploadStatusChange|FileDropzone|BulkUploadContainer' src
printf '%s\n' '--- uploader wrapper references ---'
rg -n -C 5 'uploader\.qq\.|autoUpload|uploadStoredFiles|upload\(' src/components/Upload src | sed -n '1,260p'Repository: KPMP/libra-web
Length of output: 23888
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- BulkUploadContainer.js ---'
cat -n src/components/Upload/BulkUploadContainer.js
printf '%s\n' '--- fineUploader.js ---'
cat -n src/components/Upload/fineUploader.js
printf '%s\n' '--- UploadReducer.js ---'
cat -n src/components/Upload/UploadReducer.js
printf '%s\n' '--- setIsUploading call sites ---'
rg -n -C 6 'setIsUploading|SET_IS_UPLOADING|dispatch\(' src
printf '%s\n' '--- Fine Uploader dependency/version ---'
rg -n -C 2 '"fine-uploader"|"react-fine-uploader"' package.json package-lock.json yarn.lock 2>/dev/null || true
printf '%s\n' '--- source-level invariant probe ---'
python3 - <<'PY'
from pathlib import Path
bulk = Path("src/components/Upload/BulkUpload.js").read_text()
container = Path("src/components/Upload/BulkUploadContainer.js").read_text()
wrapper = Path("src/components/Upload/fineUploader.js").read_text()
print("statusChange registration:", "uploader.on('statusChange', this.handleUploadStatusChange)" in bulk)
print("statusChange handler dispatches setIsUploading:", "setIsUploading" in bulk)
print("container maps isUploading:", "isUploading: state.isUploading" in container)
print("container maps dispatch:", "mapDispatchToProps" in container and "dispatch" in container)
print("autoUpload false:", "autoUpload: false" in wrapper)
PYRepository: KPMP/libra-web
Length of output: 11336
🌐 Web query:
Fine Uploader 5.16.2 statusChange event status constants UPLOADING SUBMITTING uploadStoredFiles
💡 Result:
In Fine Uploader 5.16.2, the statusChange event is triggered whenever an item's status changes [1][2]. The event callback provides the file ID, the old status, and the new status [1][2]. The status values correspond to the constants found in the qq.status object [1][2]. Based on official documentation for version 5.16.2, the available status constants are [1]: SUBMITTING: The file is being processed and validated after being selected [1][3]. SUBMITTED: The file has been successfully submitted and is ready for the uploader [1][3]. QUEUED: The file is waiting for an available connection to start uploading [1][3]. UPLOADING: The file is currently being uploaded [1][3]. UPLOAD_FINALIZING: The file's data has been sent and the uploader is waiting for the server's final response [1]. UPLOAD_RETRYING: The state before an automatic retry attempt begins [1][3]. UPLOAD_FAILED: The upload has failed after all retry attempts [1][3]. UPLOAD_SUCCESSFUL: The upload completed successfully [1][3]. CANCELED: The upload has been canceled [1][3]. REJECTED: The file failed validation [1][3]. DELETED: The file was successfully deleted [1][3]. DELETING: A delete attempt is in progress [1][3]. DELETE_FAILED: The last delete attempt failed [1][3]. PAUSED: The upload process is paused [1][3]. Regarding uploadStoredFiles, this is an API method rather than a status constant [4][5]. When autoUpload is set to false, files added to the uploader are not uploaded immediately [4][5]. Calling the uploadStoredFiles method manually triggers the upload process for these queued files [4][5].
Citations:
- 1: https://docs.fineuploader.com/features/statistics-and-status-updates.html
- 2: https://docs.fineuploader.com/branch/develop/api/events.html
- 3: https://docs.fineuploader.com/branch/develop/features/statistics-and-status-updates.html
- 4: https://docs.fineuploader.com/branch/develop/features/upload-files.html
- 5: https://docs.fineuploader.com/branch/master/features/upload-files.html
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- all uploader method/event call sites ---'
rg -n -C 5 'getUploader|uploader\.on|uploader\.off|uploader\.methods|uploadStoredFiles|upload\(' . -g '!*.map' -g '!package-lock.json' -g '!yarn.lock' | sed -n '1,360p'
printf '%s\n' '--- package scripts and upload-related docs ---'
cat -n package.json
rg -n -i -C 4 'bulk.?upload|uploadStoredFiles|isUploading|submit the upload|autoUpload' README* docs src 2>/dev/null | sed -n '1,260p' || true
printf '%s\n' '--- action definition ---'
cat -n src/actions/Packages/packageActions.js | sed -n '1,90p'
printf '%s\n' '--- source-level upload-start probe ---'
python3 - <<'PY'
from pathlib import Path
files = [p for p in Path('.').rglob('*') if p.is_file() and '.git' not in p.parts and p.suffix in {'.js', '.jsx', '.ts', '.tsx'}]
terms = ('uploadStoredFiles', '.upload(', 'methods.upload', 'setIsUploading')
for term in terms:
hits = []
for p in files:
text = p.read_text(errors='ignore')
if term in text:
hits.append(str(p))
print(f'{term}: {hits}')
PYRepository: KPMP/libra-web
Length of output: 17752
Connect and update the Redux upload state, and start queued uploads.
BulkUploadContainer provides no dispatch prop, and handleUploadStatusChange never dispatches setIsUploading. autoUpload is false, and handleSubmit never calls uploader.methods.uploadStoredFiles(). Map setIsUploading, dispatch the aggregate active state, and start the queued upload on submit. Otherwise FileDropzone cannot reflect upload progress and selected files remain queued.
2 participants
Summary by CodeRabbit
New Features
Style