fix: prevent saving empty links (#267)#285
Conversation
|
@prakshithamalla-art is attempting to deploy a commit to the vishnukothakapu's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Review limit reached
More reviews will be available in 52 minutes and 36 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR addresses empty link submission in the dashboard by adding client-side validation to the AddLinkBox form. The component now rejects empty/whitespace URLs with an error toast before validation, trims URLs before POST, and includes comprehensive test coverage for the new behavior plus supporting test dependencies. ChangesEmpty URL Validation and Test Coverage
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Anushreebasics
left a comment
Anushreebasics
left a comment
There was a problem hiding this comment.
Thanks for PR!
Please address the following changes:
-
Remove or relocate validateAndAddLink from [DashboardClient] (or use it in [AddLinkBox] before POST).
-
Wire isProcessing through [LinksSection]→ [AddLinkBox] to disable the add UI during processing, or remove it and rely on the existing [loading] inside [AddLinkBox]
-
Add a small unit/integration test to cover the add-link flow (client + server validation).
-
Fix newline at EOF in [DashboardClient.tsx] and confirm any stylistic text/footer changes are intended.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/dashboard/AddLinkBox.tsx (1)
85-93:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate the trimmed URL, not the raw input.
The empty-guard rejects whitespace-only input, but
validateUrl(url)then runs on the untrimmed string while the payload later sendsurl.trim(). A value like" https://example.com "passes the empty check, may be rejected byvalidateUrldue to surrounding whitespace, yet the intended (trimmed) value is valid. Trim once and validate the trimmed value to keep validation and submission consistent.🐛 Proposed fix
- if (!url || !url.trim()) { - return toast.error("Field cannot be empty"); - } - - // Structural and configuration checking parameters - const validation = validateUrl(url); + const trimmedUrl = url.trim(); + if (!trimmedUrl) { + return toast.error("Field cannot be empty"); + } + + const validation = validateUrl(trimmedUrl); if (!validation.valid) { return toast.error(validation.error); }And reuse
trimmedUrlin the payload:- url: url.trim(), + url: trimmedUrl,🤖 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 `@app/dashboard/AddLinkBox.tsx` around lines 85 - 93, Trim the input once and validate the trimmed value: create a trimmedUrl = url.trim() and use trimmedUrl for the empty check, pass trimmedUrl into validateUrl(trimmedUrl) instead of the raw url, and reuse trimmedUrl in the payload submission; update any toast.error calls to reference validation.error from validateUrl(trimmedUrl) so validation and submission are consistent (references: AddLinkBox.tsx, variable url, trimmedUrl, function validateUrl, toast.error).package.json (1)
6-14:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftFix test runner/tooling wiring for the new React test
package.json’stestscript runstsx --testonly forlib/csrf.test.ts,lib/middleware/csrf.test.ts,lib/analytics.test.ts, andlib/accountMergeUtils.test.ts—it doesn’t includeapp/dashboard/__tests__/AddLinkBox.test.tsx, so that test won’t run in CI.- Searches through JSON manifests found no Jest runner/config entries (
jest,ts-jest,babel-jest,jest-environment-jsdom); the only related testing-library item surfaced was@testing-library/dominpackage-lock.json.Wire the new test into the invoked runner (either switch to a proper Jest +
jsdomsetup, or ensure the test is compatible with thetsx --testrunner and that CI executes it).🤖 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 `@package.json` around lines 6 - 14, The test script in package.json currently invokes "tsx --test" with a hardcoded list of files and omits the new React test app/dashboard/__tests__/AddLinkBox.test.tsx; update the "test" npm script (the "test" entry) to include the new test path or replace the hardcoded list with a glob (e.g., all **/*.test.*) so the AddLinkBox.test.tsx runs in CI, and if the test requires jsdom/React support, either switch the runner to a proper Jest setup (add jest + jest-environment-jsdom and a jest config) or confirm the tsx test runner supports jsdom and adjust test tooling accordingly; ensure references to "test" script, "tsx --test", and the file app/dashboard/__tests__/AddLinkBox.test.tsx are updated consistently.
🧹 Nitpick comments (1)
app/dashboard/AddLinkBox.tsx (1)
20-25: 💤 Low valueTrim the misleading/verbose comment blocks.
Several added comments are inaccurate or noise (e.g., the JSDoc has a stray
* *on Line 23; Line 63 claims validation happens "inside local state handlers" when it's actually insubmit(); the trailing "System Maintenance Context Block" on Lines 197-200 documents nothing actionable). These add maintenance overhead without value.Also applies to: 59-65, 196-200
🤖 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 `@app/dashboard/AddLinkBox.tsx` around lines 20 - 25, Trim and correct noisy/misleading comments in AddLinkBox: remove the stray "* *" in the JSDoc block that documents the platform-key formatter and simplify that JSDoc to a single accurate sentence with correct `@param` and `@returns`, update the comment that currently claims validation happens "inside local state handlers" to correctly state that validation occurs in submit(), and delete the non-actionable "System Maintenance Context Block" comment at the end of the file; locate these edits around the platform-key formatter JSDoc and the AddLinkBox component's submit() method to ensure comments now reflect the actual implementation.
🤖 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 `@app/dashboard/__tests__/AddLinkBox.test.tsx`:
- Around line 1-25: The test file app/dashboard/__tests__/AddLinkBox.test.tsx is
not executed because the test runner only picks up lib/*.test.ts via `tsx
--test` and the test uses Jest globals; to fix, move or rename the test into the
lib test pattern (e.g., relocate AddLinkBox.test.tsx into lib/ as
lib/AddLinkBox.test.tsx or lib/__tests__/AddLinkBox.test.tsx) and update its
import path for AddLinkBox if needed so the existing jest.mock usage and
assertions remain valid; this ensures the test is discovered by the current npm
test runner without changing test framework wiring.
In `@package.json`:
- Line 54: Add `@testing-library/dom` to devDependencies in package.json alongside
`@testing-library/react`; update the devDependencies block to include the package
(matching semver compatible with `@testing-library/react`, e.g., a caret version
that aligns with the peer expected by "`@testing-library/react`": "^16.3.2") so
the project declares the peer explicitly instead of relying on package-lock.json
or auto-install behavior.
---
Outside diff comments:
In `@app/dashboard/AddLinkBox.tsx`:
- Around line 85-93: Trim the input once and validate the trimmed value: create
a trimmedUrl = url.trim() and use trimmedUrl for the empty check, pass
trimmedUrl into validateUrl(trimmedUrl) instead of the raw url, and reuse
trimmedUrl in the payload submission; update any toast.error calls to reference
validation.error from validateUrl(trimmedUrl) so validation and submission are
consistent (references: AddLinkBox.tsx, variable url, trimmedUrl, function
validateUrl, toast.error).
In `@package.json`:
- Around line 6-14: The test script in package.json currently invokes "tsx
--test" with a hardcoded list of files and omits the new React test
app/dashboard/__tests__/AddLinkBox.test.tsx; update the "test" npm script (the
"test" entry) to include the new test path or replace the hardcoded list with a
glob (e.g., all **/*.test.*) so the AddLinkBox.test.tsx runs in CI, and if the
test requires jsdom/React support, either switch the runner to a proper Jest
setup (add jest + jest-environment-jsdom and a jest config) or confirm the tsx
test runner supports jsdom and adjust test tooling accordingly; ensure
references to "test" script, "tsx --test", and the file
app/dashboard/__tests__/AddLinkBox.test.tsx are updated consistently.
---
Nitpick comments:
In `@app/dashboard/AddLinkBox.tsx`:
- Around line 20-25: Trim and correct noisy/misleading comments in AddLinkBox:
remove the stray "* *" in the JSDoc block that documents the platform-key
formatter and simplify that JSDoc to a single accurate sentence with correct
`@param` and `@returns`, update the comment that currently claims validation happens
"inside local state handlers" to correctly state that validation occurs in
submit(), and delete the non-actionable "System Maintenance Context Block"
comment at the end of the file; locate these edits around the platform-key
formatter JSDoc and the AddLinkBox component's submit() method to ensure
comments now reflect the actual implementation.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b68eca50-cc11-4f7b-91c3-51895e12005e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
app/dashboard/AddLinkBox.tsxapp/dashboard/DashboardClient.tsxapp/dashboard/__tests__/AddLinkBox.test.tsxpackage.json
3 participants
Overview
Closes #267. Added robust client-side validation to the dashboard to prevent users from submitting empty or whitespace-only links.
Changes Implemented
validateAndAddLinkinsideDashboardClient.tsxto ensure bothurlandlabelare populated before API execution..trim()on all inputs to prevent the database from being populated with whitespace-only entries.toastnotifications to provide clear error messages when validation fails.isProcessingstate to prevent duplicate form submissions during network requests.Why this improves LinkID
Summary by CodeRabbit
Bug Fixes
Tests
Chores