fix: use requestSubmit() to preserve HTML5 form validation#135
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/resources/static/js/script.js (1)
64-79:⚠️ Potential issue | 🟠 MajorForm is submitted even after upload failures.
In the failure (
else) andcatchbranches (lines 64-74) the loop only resets the button state and continues; control still reaches line 79 and the ticket form is submitted. The result: a ticket can be created where some attachments silently never made it into#hidden-file-inputs, whilestatus.innerTextis overwritten by"All uploads completed."at line 78, masking the earlier error.Consider tracking failure and either aborting the final
requestSubmit()or prompting the user before submitting:♻️ Suggested change
+ let hadFailure = false; for (let i = 0; i < filesToUpload.length; i++) { ... } else { status.innerText = `Failed to upload ${file.name}. Status: ${putRes.status}`; submitBtn.disabled = false; submitBtn.innerText = "Create Ticket"; + hadFailure = true; } } catch (error) { console.error(error); status.innerText = `Error uploading ${file.name}: ${error.message}`; submitBtn.disabled = false; submitBtn.innerText = "Create Ticket"; + hadFailure = true; } } + if (hadFailure) { + status.innerText = "Some files failed to upload. Please retry before creating the ticket."; + return; + } submitBtn.disabled = false; submitBtn.innerText = "Create Ticket"; status.innerText = "All uploads completed."; document.querySelector('form').requestSubmit();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/static/js/script.js` around lines 64 - 79, The form is always submitted after the upload loop even when individual uploads failed; add a failure-tracking boolean (e.g., uploadFailed) inside the upload loop and set it to true in the else and catch branches where putRes.status is non-OK or an exception occurs (references: submitBtn, status, requestSubmit). After the loop, check uploadFailed and if true, do not call document.querySelector('form').requestSubmit(); instead restore submitBtn.disabled and submitBtn.innerText, update status with a persistent error message, and optionally prompt the user to retry or confirm before submitting. Ensure any early-return logic or user prompt is clearly tied to uploadFailed so successful runs still call requestSubmit().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/resources/static/js/script.js`:
- Line 31: The code calls document.querySelector('form') (used in
uploadNewFile()) which can be null on upload.html and the upload loop error
handlers reset the button but do not stop the process; fix by resolving the form
from the button that triggered the action (use the triggering element's .form
property instead of document.querySelector('form')) and, inside the upload
loop/error handlers (the upload request success/error branches in
uploadNewFile), return or break immediately on any upload failure so the
function does not continue to submit the form at line 79 with missing
attachments and gives clear failure state to the user.
---
Outside diff comments:
In `@src/main/resources/static/js/script.js`:
- Around line 64-79: The form is always submitted after the upload loop even
when individual uploads failed; add a failure-tracking boolean (e.g.,
uploadFailed) inside the upload loop and set it to true in the else and catch
branches where putRes.status is non-OK or an exception occurs (references:
submitBtn, status, requestSubmit). After the loop, check uploadFailed and if
true, do not call document.querySelector('form').requestSubmit(); instead
restore submitBtn.disabled and submitBtn.innerText, update status with a
persistent error message, and optionally prompt the user to retry or confirm
before submitting. Ensure any early-return logic or user prompt is clearly tied
to uploadFailed so successful runs still call requestSubmit().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 99007d34-b573-48a4-ac41-9cb4efc84cb9
📒 Files selected for processing (1)
src/main/resources/static/js/script.js
2 participants
Summary by CodeRabbit