Add HCaptcha integration to NetworkCoverageAddMonitorDialog and update dependencies - #3621
Conversation
|
Warning Review limit reached
More reviews will be available in 27 minutes and 42 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds HCaptcha bot-verification to the NetworkCoverageAddMonitorDialog. The component now requires users to complete a captcha challenge before submitting a new network monitor, validates the captcha token alongside other form fields, includes the token in the API payload, and resets the widget after successful submission. ChangesHCaptcha Bot Verification in Network Monitor Dialog
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
|
New azure website changes available for preview here |
![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
🧹 Nitpick comments (2)
src/website/src/views/solutions/NetworkCoverage/components/NetworkCoverageAddMonitorDialog.tsx (2)
81-85: ⚡ Quick winRemove unused
scrollContainerRefcheck.The effect checks
scrollContainerRef.currentbut never uses it. ThescrollIntoViewcall only operates onerrorRef.current, so the container check is unnecessary.♻️ Simplify the condition
- if (error && errorRef.current && scrollContainerRef.current) { + if (error && errorRef.current) { errorRef.current.scrollIntoView({ behavior: 'smooth', block: 'center' }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/website/src/views/solutions/NetworkCoverage/components/NetworkCoverageAddMonitorDialog.tsx` around lines 81 - 85, The useEffect in NetworkCoverageAddMonitorDialog checks scrollContainerRef.current but never uses it; remove the unnecessary check so the effect only guards on error and errorRef.current before calling errorRef.current.scrollIntoView({ behavior: 'smooth', block: 'center' }). Update the dependency array and condition to reference only error and errorRef (keep useEffect, errorRef, and the scrollIntoView call intact) to simplify the logic.
474-474: 💤 Low valueOptional: Simplify captcha token payload assignment.
Since validation at line 419 enforces
captchaTokenas required, it will always be truthy at this point. The conditional check is technically redundant, though it follows the defensive pattern used for optional fields above.♻️ Alternative (optional)
- if (captchaToken) payload.captchaToken = captchaToken; + payload.captchaToken = captchaToken;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/website/src/views/solutions/NetworkCoverage/components/NetworkCoverageAddMonitorDialog.tsx` at line 474, In NetworkCoverageAddMonitorDialog, the conditional "if (captchaToken) payload.captchaToken = captchaToken;" is redundant because prior validation guarantees captchaToken is set; simplify by assigning directly: set payload.captchaToken = captchaToken (remove the conditional) where payload is constructed so the token is always included; ensure you only change this assignment and keep other optional-field defensive checks intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/website/src/views/solutions/NetworkCoverage/components/NetworkCoverageAddMonitorDialog.tsx`:
- Around line 938-945: The HCaptcha usage unconditionally asserts
process.env.NEXT_PUBLIC_HCAPTCHA_SITE_KEY with ! which can throw if the env var
is missing; update the NetworkCoverageAddMonitorDialog render to check the
runtime value of process.env.NEXT_PUBLIC_HCAPTCHA_SITE_KEY (used as the sitekey
prop for HCaptcha) and only render the HCaptcha component when it is a non-empty
string, otherwise render a safe fallback UI/message and ensure
handleCaptchaVerify/handleCaptchaExpire/captchaRef logic is bypassed or handles
the absence gracefully; alternatively, compute a validated const (e.g.,
captchaSiteKey) at module/component start and use that in the conditional render
to avoid the non-null assertion.
---
Nitpick comments:
In
`@src/website/src/views/solutions/NetworkCoverage/components/NetworkCoverageAddMonitorDialog.tsx`:
- Around line 81-85: The useEffect in NetworkCoverageAddMonitorDialog checks
scrollContainerRef.current but never uses it; remove the unnecessary check so
the effect only guards on error and errorRef.current before calling
errorRef.current.scrollIntoView({ behavior: 'smooth', block: 'center' }). Update
the dependency array and condition to reference only error and errorRef (keep
useEffect, errorRef, and the scrollIntoView call intact) to simplify the logic.
- Line 474: In NetworkCoverageAddMonitorDialog, the conditional "if
(captchaToken) payload.captchaToken = captchaToken;" is redundant because prior
validation guarantees captchaToken is set; simplify by assigning directly: set
payload.captchaToken = captchaToken (remove the conditional) where payload is
constructed so the token is always included; ensure you only change this
assignment and keep other optional-field defensive checks intact.
🪄 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: 9731e998-6f0d-4963-b40d-98eb3e03a605
⛔ Files ignored due to path filters (1)
src/website/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
src/website/package.jsonsrc/website/src/views/solutions/NetworkCoverage/components/NetworkCoverageAddMonitorDialog.tsx
There was a problem hiding this comment.
Pull request overview
This PR adds an hCaptcha gate to the Network Coverage “Add Monitor” dialog so that saving a network monitor requires captcha verification, and updates the website dependencies to include the hCaptcha React package.
Changes:
- Added
@hcaptcha/react-hcaptchadependency. - Integrated an hCaptcha widget into
NetworkCoverageAddMonitorDialog, including token handling and sendingcaptchaTokenin the save payload. - Added additional required-field validation and improved error display (auto-scroll to error).
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/website/src/views/solutions/NetworkCoverage/components/NetworkCoverageAddMonitorDialog.tsx | Adds hCaptcha UI + token state, expands required-field validation, and improves error visibility/scrolling. |
| src/website/package.json | Adds @hcaptcha/react-hcaptcha dependency. |
| src/website/package-lock.json | Locks @hcaptcha/react-hcaptcha and its resolved metadata. |
Files not reviewed (1)
- src/website/package-lock.json: Language not supported
Comments suppressed due to low confidence (2)
src/website/src/views/solutions/NetworkCoverage/components/NetworkCoverageAddMonitorDialog.tsx:703
- These fields are now visually marked as required ("*") but the inputs are missing
required/aria-required, so browsers and assistive tech won’t recognize them as required. Addrequiredandaria-required="true"to match the new validation rules.
<label className="block sm:col-span-2">
<div className="mb-1 text-xs font-semibold text-slate-500">
Network *
</div>
<input
value={network}
onChange={(e) => setNetwork(e.target.value)}
className="w-full rounded-md border border-slate-200 bg-white px-3 py-2 text-sm outline-none focus:border-blue-300"
/>
</label>
src/website/src/views/solutions/NetworkCoverage/components/NetworkCoverageAddMonitorDialog.tsx:748
- These fields are now visually marked as required ("*") but the inputs are missing
required/aria-required, so browsers and assistive tech won’t recognize them as required. Addrequiredandaria-required="true"to match the new validation rules.
<label className="block">
<div className="mb-1 text-xs font-semibold text-slate-500">
Manufacturer *
</div>
<input
value={manufacturer}
onChange={(e) => setManufacturer(e.target.value)}
className="w-full rounded-md border border-slate-200 bg-white px-3 py-2 text-sm outline-none focus:border-blue-300"
/>
</label>
<label className="block sm:col-span-2">
<div className="mb-1 text-xs font-semibold text-slate-500">
Pollutants (comma separated) *
</div>
<input
value={pollutants}
onChange={(e) => setPollutants(e.target.value)}
placeholder="PM2.5, PM10"
className="w-full rounded-md border border-slate-200 bg-white px-3 py-2 text-sm outline-none focus:border-blue-300"
/>
</label>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const previousActiveElementRef = useRef<HTMLElement | null>(null); | ||
| const closeTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); | ||
|
|
||
| const captchaRef = useRef<HCaptcha>(null); |
| <div className="mt-4"> | ||
| <HCaptcha | ||
| sitekey={process.env.NEXT_PUBLIC_HCAPTCHA_SITE_KEY!} | ||
| onVerify={handleCaptchaVerify} | ||
| onExpire={handleCaptchaExpire} | ||
| ref={captchaRef} | ||
| /> | ||
| </div> |
| if (!operator.trim()) return 'Operator is required'; | ||
| if (!manufacturer.trim()) return 'Manufacturer is required'; | ||
| if (!pollutants.trim()) return 'Pollutants are required'; | ||
| if (!captchaToken) return 'Please complete the captcha verification'; |
…CoverageAddMonitorDialog
|
New azure website changes available for preview here |
Summary of Changes (What does this PR do?)
Status of maturity (all need to be checked before merging):
Screenshots (optional)
Summary by CodeRabbit