fix: accept bare domains in website add dialog#793
Conversation
The domain input required a full URL (e.g. https://example.com) but the field label says "Domain name", causing confusion (#788). - Change input type from `url` to `text` to allow bare domain entry - Replace `.url()` validation with a custom `refine` that accepts both bare domains (`example.com`) and full URLs (`https://example.com`) - Add `validDomain` translation string with a helpful example - Add `deleting` website status string and scan interval translations
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughThis PR addresses domain validation inconsistency (issue Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes The diff introduces heterogeneous changes spanning validation logic, permission gating, state management with effects, UI column restructuring, and new dialog functionality. While individual components range from trivial (permission checks) to moderate (filtering, column definitions), the website-view-dialog.tsx file alone contains substantial logic density with multiple new components, state hooks, and search workflows. The variety of change types and the substantial modifications to the view dialog drive complexity despite some repetitive patterns (e.g., permission checks). Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/platform/app/features/websites/components/websites-table.tsx (1)
55-59:⚠️ Potential issue | 🔴 CriticalAdd
intervalparameter to query for server-side filtering.The
intervalprop is received and displayed in the filter UI, but it's not passed touseListWebsitesPaginated, so the data isn't actually filtered by interval. Unlikestatuswhich is properly passed to the query and filtered server-side,intervalhas no filtering implementation. Addinterval?: stringtoListWebsitesPaginatedArgs(both in the Convex handler and hooks type), pass it to the query, and update the server-side filtering logic inlist_websites_paginated.tsto filter by interval like it does for status.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/features/websites/components/websites-table.tsx` around lines 55 - 59, The paginated query is missing the interval filter: add an optional interval?: string field to the ListWebsitesPaginatedArgs type (both in the Convex handler signature and the generated hooks/types), update all callers including the use in the component where paginatedResult is created via useListWebsitesPaginated to pass the interval prop, and modify list_websites_paginated.ts server-side logic to apply the same interval-based filtering as status (filter by interval when args.interval is present before pagination). Ensure function/type names: useListWebsitesPaginated, ListWebsitesPaginatedArgs, and the handler in list_websites_paginated.ts are updated consistently so the client hook and server handler accept and use interval.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/platform/app/features/websites/components/website-view-dialog.tsx`:
- Around line 437-442: The key for the mapped SearchResultItem currently uses
`${result.url}-${result.chunk_index}-${idx}`, which redundantly includes the
index; if result.url + result.chunk_index uniquely identify each result, remove
`idx` to avoid reorder-related re-render issues. Update the map generating
SearchResultItem (the searchResults.map block) to use a stable key based only on
result.url and result.chunk_index (or another unique id field on result if
available), ensuring the key remains a string and still unique across items.
- Around line 469-471: The Button in WebsiteViewDialog currently shows a
hardcoded '...' when isPending; replace that with an accessible, localized
loading indicator by using the imported Spinner component or a translated string
(e.g., t('loading')) instead of '...'. Update the conditional rendering around
isPending in the Button so it renders <Spinner /> (with an appropriate
aria-label) or t('pagesDialog.loading') to ensure localization and accessibility
while keeping the Button text for the non-pending branch
(t('pagesDialog.loadMore')).
- Around line 227-230: When handling failures in the fetchPages onError
callback, add user-visible error feedback instead of only flipping
setIsFirstLoad; update the onError for fetchPages to call the same
toast/error-display used by searchContent (include the error message/details)
and then setIsFirstLoad(false) so failures show an error toast rather than the
misleading empty "no pages" state; locate the fetchPages onError handler and
mirror the searchContent error behavior.
- Around line 79-84: The fetchChunks useConvexAction currently only sets chunks
on success and has no error path; add error handling by providing an onError
callback to the useConvexAction for api.websites.actions.fetchChunks that sets
an error state (e.g., setFetchError) and clears or preserves chunks
appropriately, and ensure the UI uses that state to show a user-facing error
message or toast when fetch fails instead of silently disappearing (adjust the
component around setChunks, fetchChunks and isPending to render the error). Also
consider logging the error for debugging and disabling any success-only
assumptions in the component that rely on setChunks.
In `@services/platform/app/features/websites/hooks/use-websites-table-config.tsx`:
- Around line 120-136: The intervalLabels map is being recreated on every render
inside the cell renderer; move it out of the per-cell render by extracting it to
a stable lookup (e.g., a top-level constant or a useMemo inside
useWebsitesTableConfig) and use that from the cell; update the cell
implementation (the cell function in use-websites-table-config.tsx) to look up
the label via the stable map or compute via tEntity('scanIntervals.' +
row.original.scanInterval) and return intervalLabels[row.original.scanInterval]
|| row.original.scanInterval so row.original.scanInterval and tEntity are
referenced but no per-render object allocation occurs.
- Around line 42-65: The statusLabels map is being recreated on every render
inside the column's cell renderer (the cell function for accessorKey 'status'),
hurting performance; extract the labels out of the cell (either define a
top-level const in this module or compute them once with useMemo where
use-websites-table-config builds the columns) and have the cell reference that
shared statusLabels map; keep using the existing statusVariant lookup and return
logic unchanged so only the label lookup is moved from inside the cell to a
single, memoized/shared definition.
---
Outside diff comments:
In `@services/platform/app/features/websites/components/websites-table.tsx`:
- Around line 55-59: The paginated query is missing the interval filter: add an
optional interval?: string field to the ListWebsitesPaginatedArgs type (both in
the Convex handler signature and the generated hooks/types), update all callers
including the use in the component where paginatedResult is created via
useListWebsitesPaginated to pass the interval prop, and modify
list_websites_paginated.ts server-side logic to apply the same interval-based
filtering as status (filter by interval when args.interval is present before
pagination). Ensure function/type names: useListWebsitesPaginated,
ListWebsitesPaginatedArgs, and the handler in list_websites_paginated.ts are
updated consistently so the client hook and server handler accept and use
interval.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 1ee5cfd6-4ee7-44e6-a790-7aa5c06d3f91
📒 Files selected for processing (8)
services/platform/app/features/websites/components/website-add-dialog.tsxservices/platform/app/features/websites/components/website-row-actions.tsxservices/platform/app/features/websites/components/website-view-dialog.tsxservices/platform/app/features/websites/components/websites-action-menu.tsxservices/platform/app/features/websites/components/websites-table.tsxservices/platform/app/features/websites/hooks/use-websites-table-config.tsxservices/platform/app/routes/dashboard/$id/_knowledge/websites.tsxservices/platform/messages/en.json
| onError: () => { | ||
| setIsFirstLoad(false); | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Missing error feedback for page fetch failures.
When fetchPages fails, the error handler only updates isFirstLoad state. Unlike searchContent which shows a toast on error (line 242), page fetch failures silently result in the "no pages" empty state, misleading users into thinking no pages exist rather than indicating a fetch error.
🛡️ Proposed fix
onError: () => {
setIsFirstLoad(false);
+ toast({ title: t('toast.fetchPagesError'), variant: 'destructive' });
},📝 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.
| onError: () => { | |
| setIsFirstLoad(false); | |
| }, | |
| }, | |
| onError: () => { | |
| setIsFirstLoad(false); | |
| toast({ title: t('toast.fetchPagesError'), variant: 'destructive' }); | |
| }, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/websites/components/website-view-dialog.tsx`
around lines 227 - 230, When handling failures in the fetchPages onError
callback, add user-visible error feedback instead of only flipping
setIsFirstLoad; update the onError for fetchPages to call the same
toast/error-display used by searchContent (include the error message/details)
and then setIsFirstLoad(false) so failures show an error toast rather than the
misleading empty "no pages" state; locate the fetchPages onError handler and
mirror the searchContent error behavior.
1 participant
Fixes #788
Summary
urltotextto allow bare domain entry (e.g.example.com).url()Zod validation with a customrefinethat accepts both bare domains and full URLsvalidDomaintranslation string with a helpful placeholder exampledeletingwebsite status and scan interval label translationsTest plan
example.com(no protocol) → succeedshttps://example.com→ succeedsnotadomain→ shows "Enter a valid domain (e.g. example.com)" error🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
UI/UX Improvements