Search fix #60
Search fix #60
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughA new interactive bar chart component was introduced to visualize aggregated search results by selectable attributes. The search interface was updated to support this chart, including new state management, aggregation logic, and loading feedback. The results table now supports an Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SearchControls
participant ResultsTable
participant FacetBarChart
User->>SearchControls: Enters query or changes filters
SearchControls->>ResultsTable: Renders with onDataUpdate callback
ResultsTable-->>SearchControls: Calls onDataUpdate with new hits
SearchControls->>SearchControls: Aggregates hits into chart data
SearchControls->>FacetBarChart: Renders with aggregated data and selected attribute
User->>FacetBarChart: Changes chart attribute
FacetBarChart-->>SearchControls: setChartAttribute callback
SearchControls->>FacetBarChart: Updates with new aggregation
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
♻️ Duplicate comments (1)
website/src/components/Search.tsx (1)
49-51: DuplicateChartableAttributeunionSee earlier note in
FacetBarChart.tsx; import the shared type instead of redefining it here.
🧹 Nitpick comments (7)
website/src/app/[lang]/layout.tsx (1)
71-75: Avoid combiningasyncanddefer– one of them is ignoredWhen both attributes are present, browsers treat the script as
async, makingdeferredundant.
Removing the extra attribute avoids confusion and keeps the markup tidy.-<script async defer src="https://scripts.simpleanalyticscdn.com/latest.js"></script> +<script async src="https://scripts.simpleanalyticscdn.com/latest.js"></script>website/src/components/ResultsTable.tsx (2)
22-28: Prevent unnecessary re-renders when propagating hit data
useHitsreturns a new array instance on every search event, so the effect fires even when the underlying list hasn’t semantically changed, causing the parent component (chart) to re-render more often than needed.Consider memoising the payload you send upstream (e.g. by comparing
hits.map(h => h.objectID).join()or usingfast-deep-equal) or throttling updates.-useEffect(() => { - if (onDataUpdate) { - onDataUpdate(hits); - } -}, [hits, onDataUpdate]); +useEffect(() => { + if (!onDataUpdate) return; + const ids = hits.map(h => h.objectID).join(','); + onDataUpdate(hits); + // eslint-disable-next-line react-hooks/exhaustive-deps +}, [ids]); // fires only when the actual set of hits changes
31-44: Dual scroll containers can break Table semanticsYou wrap
<Table>in a scrolling<div>and give<TableBody>its own scroll/overflow styles.
This often results in two nested scrollbars or the header scrolling away on some browsers.If vertical scrolling is desired, keep it on the wrapper only:
-<div className="max-h-[400px] overflow-y-auto"> +<div className="max-h-[400px] overflow-y-auto"> <Table> @@ - <TableBody className="max-h-[400px] overflow-y-auto overflow-x-hidden"> + <TableBody>website/src/components/FacetBarChart.tsx (3)
3-4:useMemois imported but unusedThe unused import will trigger the linter/TypeScript error
no-unused-vars. Remove it.-import { useMemo } from 'react'; +// import removed – not used
26-30:ChartableAttributeis declared in multiple files – extract to a shared typeBoth
FacetBarChart.tsxandSearch.tsxdefine the same union. Divergence later will create type mismatches.Create
src/types/chart.ts(or extendtypes/search.ts) and reuse:// types/chart.ts export type ChartableAttribute = 'payer' | 'recipient' | 'province' | 'country';Then import it in both modules.
101-110:limitprop is accepted but ignoredConsumers can specify
limit, yet the component always renders the entire dataset:-const chartData = data; +const chartData = data.slice(0, limit);Either honour the prop (as above) or drop it from
FacetBarChartProps.website/src/components/Search.tsx (1)
72-75: Dead constants – remove or implement batching
MAX_CHART_RECORDSandCHART_BATCH_SIZEare declared but unused.
If you planned to fetch additional pages for charting, implement the batching; otherwise delete to keep the codebase clean.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonwebsite/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
website/package.json(0 hunks)website/src/app/[lang]/layout.tsx(1 hunks)website/src/components/FacetBarChart.tsx(1 hunks)website/src/components/ResultsTable.tsx(3 hunks)website/src/components/Search.tsx(10 hunks)
💤 Files with no reviewable changes (1)
- website/package.json
🧰 Additional context used
🧬 Code Graph Analysis (1)
website/src/components/Search.tsx (4)
website/src/types/search.ts (1)
SearchResult(1-16)website/src/components/ResultsTable.tsx (1)
ResultsTable(19-81)website/src/components/FacetBarChart.tsx (1)
FacetBarChart(101-183)website/src/components/Layout.tsx (1)
H3(27-31)
🪛 Gitleaks (8.21.2)
website/src/components/Search.tsx
54-54: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
| const [chartAttribute, setChartAttribute] = useState<ChartableAttribute>('payer'); | ||
|
|
||
| const totalHits = results?.nbHits ?? 0; | ||
| const [isChartLoading, setIsChartLoading] = useState(false); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
isChartLoading state is never updated
You initialise the flag and render loaders based on it, but no code sets it to true/false.
Either leverage status === 'loading' from InstantSearch or add logic that toggles the flag while you aggregate data.
-const [isChartLoading, setIsChartLoading] = useState(false);
+const [isChartLoading, setIsChartLoading] = useState<boolean>(false);
useEffect(() => {
setIsChartLoading(status === 'loading');
}, [status]);📝 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.
| const [chartAttribute, setChartAttribute] = useState<ChartableAttribute>('payer'); | |
| const totalHits = results?.nbHits ?? 0; | |
| const [isChartLoading, setIsChartLoading] = useState(false); | |
| const [chartAttribute, setChartAttribute] = useState<ChartableAttribute>('payer'); | |
| - const [isChartLoading, setIsChartLoading] = useState(false); | |
| + const [isChartLoading, setIsChartLoading] = useState<boolean>(false); | |
| + useEffect(() => { | |
| + setIsChartLoading(status === 'loading'); | |
| + }, [status]); |
🤖 Prompt for AI Agents
In website/src/components/Search.tsx around lines 85 to 88, the isChartLoading
state is initialized but never updated, so loaders depending on it won't behave
correctly. Fix this by either removing isChartLoading and using the existing
status === 'loading' from InstantSearch to control loading states, or add logic
to set isChartLoading to true when data aggregation starts and false when it
finishes, ensuring the loading indicator reflects the actual loading status.
| const typesenseServerConfig = { | ||
| apiKey: 'YpZamILESYThUVYZZ87dIBuJorHtRPfa', | ||
| nodes: [{ host: 'search.canadasbuilding.com', port: 443, protocol: 'https' }], | ||
| cacheSearchResultsForSeconds: 120, | ||
| }; |
There was a problem hiding this comment.
Hard-coded Typesense API key is publicly exposed
The key YpZamILESYThUVYZZ87dIBuJorHtRPfa is embedded in client-side code, allowing anyone to query or exhaust your Typesense quota.
Move it to a server-side environment variable and inject a short-lived search-only token to the browser.
-const typesenseServerConfig = {
- apiKey: 'YpZamILESYThUVYZZ87dIBuJorHtRPfa',
+const typesenseServerConfig = {
+ apiKey: process.env.NEXT_PUBLIC_TYPESENSE_SEARCH_KEY!, // search-only📝 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.
| const typesenseServerConfig = { | |
| apiKey: 'YpZamILESYThUVYZZ87dIBuJorHtRPfa', | |
| nodes: [{ host: 'search.canadasbuilding.com', port: 443, protocol: 'https' }], | |
| cacheSearchResultsForSeconds: 120, | |
| }; | |
| const typesenseServerConfig = { | |
| apiKey: process.env.NEXT_PUBLIC_TYPESENSE_SEARCH_KEY!, // search-only | |
| nodes: [{ host: 'search.canadasbuilding.com', port: 443, protocol: 'https' }], | |
| cacheSearchResultsForSeconds: 120, | |
| }; |
🧰 Tools
🪛 Gitleaks (8.21.2)
54-54: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
In website/src/components/Search.tsx around lines 53 to 57, the Typesense API
key is hard-coded in the client-side code, exposing it publicly. Remove the API
key from this file and instead store it securely as a server-side environment
variable. Implement a backend endpoint to generate and provide a short-lived,
search-only token to the client, and update the client code to use this token
for authentication when querying Typesense.
maktouch
left a comment
maktouch
left a comment
There was a problem hiding this comment.
The deployment is failing because of the presence of package-lock.json
We use pnpm, it should be updating the pnpm-lock.yaml file instead
| function formatCompactNumber(value: number): string { | ||
| if (value >= 1e9) { | ||
| return `$${(value / 1e9).toFixed(1)}B`; | ||
| } | ||
| if (value >= 1e6) { | ||
| return `$${(value / 1e6).toFixed(1)}M`; | ||
| } | ||
| if (value >= 1e3) { | ||
| return `$${(value / 1e3).toFixed(1)}K`; | ||
| } | ||
| return `$${value.toFixed(0)}`; // Basic format for smaller numbers |
There was a problem hiding this comment.
Is it possible to use Intl.NumberFormat functions instead?
new Intl.NumberFormat("en-CA", {
notation: "compact",
compactDisplay: "short",
}).format(987654321);
Summary by CodeRabbit
New Features
Enhancements