Copy portaljs-frontend-starter#4
Conversation
Update to upstream datopian repo
|
@benhur07b is attempting to deploy a commit to the Datopian Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR rebrands and rearchitects the PortalJS/CKAN frontend: pages migrate from static generation to SSR, dataset/org/group queries switch to ChangesCore Portal Rewrite
Accessibility CI Pipeline and Docs
Estimated code review effort: 5 (Critical) | ~150 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant NextApp
participant Playwright
participant Pa11yCI
participant LighthouseCI
GitHubActions->>NextApp: build and start server
GitHubActions->>Playwright: run tests/a11y.spec.ts against __routes.json
GitHubActions->>Pa11yCI: run scripts/run-pa11y-from-routes.js
GitHubActions->>LighthouseCI: run scripts/run-lhci-from-routes.js
Playwright-->>GitHubActions: playwright-report
Pa11yCI-->>GitHubActions: pa11y-report.ndjson
LighthouseCI-->>GitHubActions: lhci-report
sequenceDiagram
participant User
participant SearchPage
participant SearchStateProvider
participant SWR
participant CKAN
User->>SearchPage: visit /search with query params
SearchPage->>SearchStateProvider: initialize from router.query
SearchStateProvider->>SWR: fetch searchDatasets(options)
SWR->>CKAN: package_search request
CKAN-->>SWR: results and facets
SWR-->>SearchStateProvider: searchResults, searchFacets
SearchStateProvider-->>SearchPage: options, results, setOptions
User->>SearchPage: apply filter or pagination
SearchPage->>SearchStateProvider: setOptions(updated query)
SearchStateProvider->>SWR: refetch with new key
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
lib/queries/orgs.ts (1)
6-19: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle missing organizations before dereferencing
org.
CkanRequest.getthrows on CKAN failures, sogetOrganization()can abort SSR before the 404 path runs. Even if it returnednull,pages/[org]/index.tsxstill readsorg.package_countandorg.namebeforeif (!org), which will crash the render instead of returningnotFound.🤖 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 `@lib/queries/orgs.ts` around lines 6 - 19, Handle missing organizations before any `org` dereference: update `getOrganization()` in `lib/queries/orgs.ts` so CKAN lookup failures for absent orgs don’t abort SSR, and make `pages/[org]/index.tsx` guard the result before reading `org.package_count` or `org.name`. Use the existing `getOrganization`/page data-loading flow to return a null/undefined org and move the `notFound` check ahead of any property access.pages/groups/[groupName].tsx (2)
16-57: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNo error handling around external CKAN calls.
Same gap as the organization page —
getGroup,searchDatasets, andckan.getGroupActivityStreamare unguarded awaits with no try/catch.🤖 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 `@pages/groups/`[groupName].tsx around lines 16 - 57, Wrap the external CKAN awaits in getServerSideProps for getGroup, searchDatasets, and ckan.getGroupActivityStream in try/catch so failures return a safe notFound or error fallback instead of crashing the page. Use the existing getServerSideProps flow in pages/groups/[groupName].tsx, keep the groupName check early, and make sure any exception from group fetching, dataset search, or activity stream retrieval is handled before building props.
25-50: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMove the
notFoundguard before usinggroup
group.package_countandckan.getGroupActivityStream(group.name)run before theif (!group)check, so a missing group still throws instead of returning a 404. Move the guard immediately aftergetGroup().🤖 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 `@pages/groups/`[groupName].tsx around lines 25 - 50, The `notFound` check in the group page flow is too late, since `group.package_count` and `ckan.getGroupActivityStream(group.name)` use `group` before verifying it exists. In the `getGroup()` handling inside `[groupName].tsx`, move the `if (!group)` guard immediately after `getGroup()` and before any access to `group.name` or `group.package_count`, then keep the dataset/activity fetches only in the valid-group path.
🟠 Major comments (38)
pages/api/queryless-chat.ts-48-61 (1)
48-61: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winValidate each forwarded message at runtime.
The cast does not prevent direct callers from sending
role: "system"or non-stringcontent, which is then forwarded to the model. Reject invalid message objects before building the upstream payload.Proposed fix
- if (!Array.isArray(messages)) { - res.status(400).json({ error: "Invalid request: messages must be an array" }); + if ( + !Array.isArray(messages) || + messages.some( + message => + !message || + (message.role !== "user" && message.role !== "assistant") || + typeof message.content !== "string" + ) + ) { + res.status(400).json({ error: "Invalid request: messages must be user/assistant text messages" }); return; }Also applies to: 87-93
🤖 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 `@pages/api/queryless-chat.ts` around lines 48 - 61, Validate each entry in the messages array at runtime in queryless-chat before constructing the upstream payload, since the RequestBody cast only enforces types at compile time. Add per-message validation to reject any object with an unsupported role such as system or non-string content, and return a 400 error before forwarding. Update the message handling path around the RequestBody destructuring and the payload-building logic used later in the handler.pages/api/queryless-chat.ts-17-20 (1)
17-20: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftEnforce rate limiting on the API route, not only in localStorage.
The browser limits in
QuerylessAssistantare bypassable; anyone can POST directly to this route and consume the server-side Queryless token. Add server-side per-IP/session limits before the upstreamfetch.Also applies to: 77-95
🤖 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 `@pages/api/queryless-chat.ts` around lines 17 - 20, The queryless chat API handler currently trusts only client-side limits, so requests can bypass `QuerylessAssistant` and hit the upstream token endpoint directly. Add server-side rate limiting in `handler` before the upstream `fetch`, using a per-IP or per-session check in this route. Locate the logic in `handler` for `pages/api/queryless-chat.ts` and enforce the limit early, returning an error response when the threshold is exceeded.components/queryless/QuerylessAssistant.tsx-955-1003 (1)
955-1003: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMove focus into the drawer when it opens.
The trigger opens a
role="dialog"but focus remains on the background button, andaria-modal="false"makes keyboard navigation ambiguous. Focus the close/input control on open and restore focus to the trigger on close, or use an accessible dialog primitive.🤖 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 `@components/queryless/QuerylessAssistant.tsx` around lines 955 - 1003, The AI assistant drawer opens without moving focus, and the current dialog setup in QuerylessAssistant leaves keyboard users on the background trigger with ambiguous modal behavior. Update the open/close flow in QuerylessAssistant so opening the drawer programmatically focuses an appropriate control inside the panel (such as the Close button or chat input), and closing it restores focus to the Ask AI trigger; alternatively, replace the custom aside/dialog handling with an accessible dialog primitive that manages focus and modal semantics for you.pages/api/queryless-chat.ts-107-110 (1)
107-110: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not echo raw upstream error bodies to clients.
detailscan include provider diagnostics, prompts, or internal context. Log a sanitized summary server-side and return a generic failure message/client-safe code.Also applies to: 160-163
🤖 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 `@pages/api/queryless-chat.ts` around lines 107 - 110, In the queryless chat API handler, stop returning the raw upstream `details` payload from `res.status(...).json(...)` because it may expose provider diagnostics or internal context. Update the response logic in the `queryless-chat` request flow to send only a generic client-safe error message or code, and move the full upstream payload to server-side logging in a sanitized form. Apply the same fix to the other error-response block referenced in the handler so both paths use the same safe pattern.package.json-52-53 (1)
52-53: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAlign TypeScript with Zod v4
package.json:52-81
zodis bumped to4.3.6, buttypescriptis still pinned to4.7.4. Zod v4 expects TypeScript 5.5+, and this mismatch can breakz.infer/schema typings for the new Dataset/Resource interfaces. Upgrade TypeScript or keep Zod on v3.🤖 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 52 - 53, TypeScript in package.json is still pinned to 4.7.4 while zod has been upgraded to 4.3.6, so align the toolchain by either upgrading TypeScript to a Zod v4-compatible 5.5+ version or downgrading zod back to v3. Update the package.json dependency entry for typescript (and any related lockfile entries) so the schema typings used by z.infer and the Dataset/Resource interfaces resolve correctly.components/_shared/MarkdownRenderer.tsx-30-37 (1)
30-37: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
inlineis no longer passed to react-markdown’scoderenderer, so inline code renders as a block.
components/_shared/MarkdownRenderer.tsx:30-37
Move the block styling topreand keepcodefor inline text; otherwise inline snippets will always use the full-width<pre><code>branch.🤖 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 `@components/_shared/MarkdownRenderer.tsx` around lines 30 - 37, In MarkdownRenderer’s react-markdown code renderer, stop relying on the removed inline flag inside the code callback because it now causes inline snippets to fall through to the block branch. Update the code and pre handling so the code renderer is used for inline text styling, while the pre element carries the block styling for fenced code, keeping the existing component structure in MarkdownRenderer.lib/queries/orgs.ts-21-29 (1)
21-29: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSignature change breaks
pages/organizations.tsx.getAllOrganizationsnow takes no arguments, butpages/organizations.tsx:12still callsgetAllOrganizations({ detailed: true }), which will fail TypeScript checking. Drop the argument there or keep an optional param for compatibility.🤖 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 `@lib/queries/orgs.ts` around lines 21 - 29, getAllOrganizations in orgs.ts no longer accepts a parameter, but pages/organizations.tsx still calls it with an object argument. Update the call site to invoke getAllOrganizations with no arguments, or if backward compatibility is needed, restore an optional parameter on getAllOrganizations and ignore it so both usages type-check. Use the unique symbol getAllOrganizations to locate the API and the pages/organizations.tsx caller.components/theme/theme-provider.tsx-23-43 (1)
23-43: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
switchThemeupdates state but rendered theme never changes.
themeDefinition(Line 31) is computed from thethemeNameprop, not from thethemestate.switchTheme(Lines 27-29) updatesthemeviasetTheme, but theHeader/Sidebar/Footer/ThemeBaserendered below (Lines 32, 36-42) still usethemeDefinition, which never changes after mount. Consumers callingsetThemefromuseTheme()will see the context'sthemevalue update, but the actual rendered layout/header/footer stays frozen on the initial prop.🐛 Proposed fix: derive themeDefinition from state
const [theme, setTheme] = useState<Theme>( themes[themeName] || themes.default ); const switchTheme = (themeName: string) => { setTheme(themes[themeName] || themes.default); }; - const themeDefinition = themes[themeName] || themes.default; - const ThemeBase = themeDefinition.layout || _ThemeBase; + const ThemeBase = theme.layout || _ThemeBase; return ( <ThemeContext.Provider value={{ theme, setTheme: switchTheme }}> <ThemeBase - Header={themeDefinition?.header} - Sidebar={themeDefinition?.sidebar} - Footer={themeDefinition?.footer} + Header={theme?.header} + Sidebar={theme?.sidebar} + Footer={theme?.footer} >🤖 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 `@components/theme/theme-provider.tsx` around lines 23 - 43, The ThemeProvider is reading rendered theme parts from the incoming themeName prop instead of the mutable theme state, so switchTheme updates context but not the UI. Update ThemeProvider to derive themeDefinition, ThemeBase, and the Header/Sidebar/Footer props from the theme state set by switchTheme, and ensure useTheme consumers drive the same state that is used for rendering.themes/default/layout.tsx-9-19 (1)
9-19: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake
DefaultThemeprops optional
theme-provider.tsxpassesthemeDefinition?.header,?.sidebar, and?.footer, soDefaultThemecan’t requireFCprops here.themes/default/index.tsxalso omitssidebar, which leaves this layout contract inconsistent.Proposed fix
}: { - Header: FC; - Sidebar: FC; - Footer: FC; + Header?: FC; + Sidebar?: FC; + Footer?: FC; children: ReactNode; }) => {🤖 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 `@themes/default/layout.tsx` around lines 9 - 19, DefaultTheme currently requires Header, Sidebar, and Footer as FC props even though theme-provider.tsx passes optional values and themes/default/index.tsx may omit sidebar, so make the DefaultTheme prop contract optional and handle missing sections safely. Update the DefaultTheme component signature in layout.tsx and keep the default theme export consistent so Header, Sidebar, and Footer can be absent without breaking the layout.components/_shared/DatasetList.tsx-24-73 (1)
24-73: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSurface SWR fetch errors instead of treating them as “no results.”
useSWRhere discardserror, so a failedsearchDatasetscall (network/CKAN outage) falls straight through to thedatasets.length === 0branch and renders "No datasets found." — indistinguishable from a legitimately empty org/group. Users and devs lose visibility into actual failures.🔧 Suggested fix
- const { data: searchResults, isValidating } = useSWR( + const { data: searchResults, isValidating, error } = useSWR( ["entity_package_search", { fq, offset, limit }], async (api, options) => { return searchDatasets({ @@ } ); const datasets = searchResults?.results || []; const count = searchResults?.count || 0; @@ + if (error) { + return ( + <div className="py-8 w-full flex justify-center"> + <span className="text-red-500">Failed to load datasets.</span> + </div> + ); + } + if (isValidating && datasets.length === 0) {🤖 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 `@components/_shared/DatasetList.tsx` around lines 24 - 73, The DatasetList SWR state is treating fetch failures as empty results because useSWR only destructures data and isValidating, so searchDatasets errors fall through to the “No datasets found” UI. Update the useSWR call in DatasetList to also capture error, then render an error state when searchDatasets fails instead of the empty-state branch; keep the existing loading and no-results behavior only for non-error cases.components/dataset/_shared/FormatsColors.tsx-35-42 (1)
35-42: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winStatic class names or a safelist are needed here.
text-[${...}]andbg-[${...}]won’t be emitted by Tailwind unless each concrete class is written literally or added totailwind.config.js.🤖 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 `@components/dataset/_shared/FormatsColors.tsx` around lines 35 - 42, The dynamic Tailwind class construction in FormatsColors.tsx via resourceTextColors/resourceBgColors will not be picked up by Tailwind’s scanner. Replace the runtime template-string generation in the resourceFormatColors loop with literal class names for each format, or add the exact text-[...] and bg-[...] variants to the Tailwind safelist/config so the styles are emitted reliably.components/dataset/individualPage/DatasetInfo.tsx-24-25 (1)
24-25: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRegex-based HTML stripping can corrupt legitimate content.
/<\/?[^>]+(>|$)/gremoves any substring starting with<up to the next>, not just HTML tags. This can silently strip valid content such as markdown autolinks (<https://example.com>) or plain text containing comparison operators (e.g.,x < 5 and y > 3), since the regex doesn't distinguish real tags from arbitrary<...>sequences.Consider using a proper HTML sanitizer (e.g.,
sanitize-html/DOMPurify) or relying onMarkdownRenderer's own sanitization instead of a hand-rolled regex.🤖 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 `@components/dataset/individualPage/DatasetInfo.tsx` around lines 24 - 25, The description cleanup in DatasetInfo should not use the hand-rolled regex in the description assignment, since it can strip valid text like autolinks or comparison operators. Replace the regex-based stripping with a proper sanitizer or trust the existing MarkdownRenderer sanitization path, and keep the fallback behavior for missing notes when computing the description.components/dataset/_shared/ResourcesBadges.tsx-16-18 (1)
16-18: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMissing guard for optional
resourcesprop.Same issue as
MultipleResourcesCard:resourcesis optional butresources.map(...)is called unconditionally, causing a crash if the prop is omitted.🐛 Proposed fix
const _unique_resources = Array.from( - new Map(resources.map((item) => [item.format, item])).values() + new Map((resources ?? []).map((item) => [item.format, item])).values() );🤖 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 `@components/dataset/_shared/ResourcesBadges.tsx` around lines 16 - 18, The ResourcesBadges component is calling resources.map unconditionally even though resources is optional, which can crash when the prop is missing. Update the logic around the _unique_resources computation to safely handle an undefined resources prop, using the same null/empty fallback approach used in MultipleResourcesCard so the Map construction only runs when resources is present.components/dataset/_shared/MultipleResourcesCard.tsx-4-9 (1)
4-9: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMissing guard for optional
resourcesprop.
resourcesis typedresources?: Resource[], but Line 9 destructures it directly (const [firstResource, ...rest] = resources;). If a caller omitsresources(as the type permits), this throws a TypeError at render time.🐛 Proposed fix
}: { resources?: Resource[]; }) { - const [firstResource, ...rest] = resources; + const [firstResource, ...rest] = resources ?? [];🤖 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 `@components/dataset/_shared/MultipleResourcesCard.tsx` around lines 4 - 9, The MultipleResourcesCard component destructures the optional resources prop without a fallback, so it can throw when resources is omitted. Update MultipleResourcesCard to guard against undefined by defaulting resources to an empty array (or returning early when absent) before the array destructuring, so firstResource and rest are only derived from a defined Resource[].components/dataset/_shared/MultipleResourcesCard.tsx-40-47 (1)
40-47: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
window.innerWidthcomputed in render body risks SSR hydration mismatch.This project uses SSR pages (per PR summary). Computing
offsetfromwindow.innerWidthdirectly in the render body means the server markup (window undefined → offset 0) can differ from the client's first render, triggering a React hydration mismatch. Prefer Tailwind responsive classes (sm:/md:variants fortop/left) or compute the offset in auseEffect+ state after mount.♻️ Suggested approach
- {visibleLayers.map((_, index) => { - //const - const offset = - typeof window !== "undefined" - ? window.innerWidth < 768 - ? (index + 1) * 4 - : (index + 1) * 6 - : 0; - return ( - <div - key={index} - style={{ - top: `${offset}px`, - left: `${offset}px`, - zIndex: 5 - index, - }} - className={`absolute w-16 md:w-20 h-16 md:h-20 bg-[var(--dark)] border border-white rounded-lg shadow-lg`} - /> - ); - })} + {visibleLayers.map((_, index) => ( + <div + key={index} + style={{ zIndex: 5 - index }} + className={`absolute w-16 md:w-20 h-16 md:h-20 bg-[var(--dark)] border border-white rounded-lg shadow-lg`} + // use responsive tailwind translate utilities instead of JS offset, e.g. translate-x-1 md:translate-x-1.5 + /> + ))}🤖 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 `@components/dataset/_shared/MultipleResourcesCard.tsx` around lines 40 - 47, The offset calculation inside MultipleResourcesCard’s render path is reading window.innerWidth directly, which can make SSR and client markup differ on first render. Update the visibleLayers.map render logic to avoid browser-only width checks during render, either by using responsive Tailwind classes for the offset positioning or by moving the width-dependent value into state set after mount in a useEffect. Keep the fix localized to MultipleResourcesCard and the offset/visibleLayers mapping so the initial server and client render stay consistent.components/dataset/search/DatasetItem.tsx-25-25 (1)
25-25: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard
dataset.organizationinDatasetItem.tsx.Dataset.organizationis optional inschemas/dataset.interface.ts, sodataset.organization.nameanddataset.organization.titlecan throw during search rendering. Use the optional chain/fallback already used elsewhere in the codebase.🤖 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 `@components/dataset/search/DatasetItem.tsx` at line 25, DatasetItem is dereferencing an optional organization object during search rendering, which can throw when dataset.organization is missing. Update the DatasetItem component to guard dataset.organization before accessing name/title, and use the same optional chaining and fallback pattern already used elsewhere in the codebase for dataset links and labels.components/dataset/search/ListOfDatasets.tsx-204-217 (1)
204-217: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake the no-results state reachable.
ResultsNotFoundis returned after an earlierreturn, so zero-result searches currently render nothing.Proposed fix
if (count > 0) { return ( <Pagination @@ /> ); - - return <ResultsNotFound />; } - // make a pagination component once insights are added - return null; + return <ResultsNotFound />; @@ - Clear fitlers + Clear filtersAlso applies to: 249-249
🤖 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 `@components/dataset/search/ListOfDatasets.tsx` around lines 204 - 217, The no-results state in ListOfDatasets is unreachable because ResultsNotFound is placed after the Pagination return inside the count > 0 branch. Update the conditional flow in ListOfDatasets so that zero-count searches explicitly return ResultsNotFound, while positive counts still render Pagination; keep the fix localized to the existing count check and the ListOfDatasets component render logic.components/dataset/search/ListOfDatasets.tsx-169-189 (1)
169-189: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRender clickable controls as buttons.
The clear-all, clear-filters, and active-filter controls are currently non-semantic clickable elements, so keyboard users cannot reliably activate them.
Also applies to: 244-251, 264-278
🤖 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 `@components/dataset/search/ListOfDatasets.tsx` around lines 169 - 189, The clear-all, clear-filters, and active-filter controls in ListOfDatasets are implemented as clickable non-semantic elements, which breaks keyboard accessibility. Refactor the clickable wrappers in ListOfDatasets so these actions use proper button elements (including the clear-all control and the active filter chips mentioned in the same component), and keep the existing click handlers and styling behavior intact by moving the interaction onto the button-based controls.components/dataset/search/SearchContext.tsx-64-78 (1)
64-78: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winAlign the SWR key with the SSR fallback or the first search refetches unnecessarily.
pages/search.tsxseeds fallback with["package_search", initialRequestOption], but this provider requests["package_search", packagesOptions]with extra default fields. The fallback andfacetsprop are therefore bypassed on first render.One possible fix
- const packageSearchFacets = packageSearchResults?.search_facets ?? {}; + const packageSearchFacets = packageSearchResults?.search_facets ?? facets ?? {};Also normalize
pages/search.tsxto use the same defaultquery,sort, andtypefields aspackagesOptions.Also applies to: 106-112
🤖 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 `@components/dataset/search/SearchContext.tsx` around lines 64 - 78, The SWR cache key in SearchContext’s package search setup does not match the SSR fallback shape, so the initial result is treated as a miss and refetches. Make the key used in useSWR(["package_search", ...]) match the object seeded in pages/search.tsx by normalizing both sides to the same request options, including the default query, sort, type, and offset handling in packagesOptions / initialRequestOption. Also ensure the facets prop derives from the same normalized search params so the first render can reuse the fallback instead of bypassing it.components/dataset/search/DatasetSearchForm.tsx-1-13 (1)
1-13: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep the query input synced and reset pagination on search.
After filters or
/searchclearing updateoptions.query, localqcan stay stale. Also, submitting a new term from page N keeps the old offset.Proposed fix
-import { useState } from "react"; +import { useEffect, useState } from "react"; @@ const { setOptions, options } = useSearchState(); const [q, setQ] = useState(options.query ?? ""); + useEffect(() => { + setQ(options.query ?? ""); + }, [options.query]); const handleSubmit = (e) => { e.preventDefault(); setOptions({ query: q, + offset: 0, });Also applies to: 25-26
🤖 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 `@components/dataset/search/DatasetSearchForm.tsx` around lines 1 - 13, DatasetSearchForm keeps a local q state that can drift from options.query, and handleSubmit currently only updates the query without resetting pagination. Update DatasetSearchForm to mirror options.query back into q whenever the search state changes, and adjust handleSubmit to clear the current page/offset when submitting a new search term. Use the existing useSearchState, setOptions, and handleSubmit logic as the hook points for this sync and reset behavior.components/dataset/search/ListOfDatasets.tsx-120-177 (1)
120-177: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReset pagination when removing active filter badges.
Facet checkboxes reset
offset, but badge removals and “clear all” do not. Removing filters from a later page can keep users on an empty offset.Proposed fix
setOptions({ orgs: options.orgs.filter((item) => item !== org.name), + offset: 0, }); @@ setOptions({ groups: options.groups.filter((item) => item !== g.name), + offset: 0, }); @@ setOptions({ tags: options.tags.filter((item) => item !== t.name), + offset: 0, }); @@ setOptions({ resFormat: options.resFormat.filter( (item) => item !== f.name ), + offset: 0, }); @@ setOptions({ resFormat: [], groups: [], orgs: [], - tags: [] + tags: [], + offset: 0, });🤖 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 `@components/dataset/search/ListOfDatasets.tsx` around lines 120 - 177, Reset pagination whenever an active filter badge is removed or “clear all” is used in ListOfDatasets. The badge onClick handlers and the activeFiltersCount clear action currently call setOptions without updating offset, unlike the facet checkbox flow; update these handlers to also reset offset back to the first page while preserving the existing filter removals. Use the existing setOptions logic in ListOfDatasets and the ActiveFilter click handlers as the main places to fix..github/workflows/accessibility.yml-39-41 (1)
39-41: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAdd
start-server-and-testtodevDependencies
npm i -D start-server-and-testin CI installs the latest release at runtime and bypasses the lockfile. Add it todevDependenciesand drop the ad hoc install sonpm cipins the version used by the workflow.🤖 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 @.github/workflows/accessibility.yml around lines 39 - 41, The accessibility workflow is installing start-server-and-test ad hoc during CI instead of using the locked package version. Add start-server-and-test to devDependencies in package management for the project, then remove the inline npm i -D start-server-and-test step so the workflow relies on npm ci and the pinned version when running npx start-server-and-test.Source: Linters/SAST tools
pages/groups/[groupName].tsx-63-66 (1)
63-66: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winComposed
namecan become"undefined--<name>".
group?.groups[0]?.name+'--'+group.nameyields the literal string"undefined"prefix whenever the group has no parent group ingroup.groups. Given the adjacent TODO already flags this as an interim hack, worth tightening before this ships broadly — e.g. fall back togroup.namealone when there's no parent.Want me to draft a fix for the fallback naming logic?
🤖 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 `@pages/groups/`[groupName].tsx around lines 63 - 66, The composed DatasetList name in the group page can produce an "undefined--..." prefix when there is no parent group. Update the name construction in the group view around DatasetList so it uses the parent group name only when group.groups[0]?.name exists, and otherwise falls back to group.name alone. Use the existing group and group.groups access pattern to keep the change localized.pages/[org]/index.tsx-18-58 (1)
18-58: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNo error handling around external CKAN calls.
getOrganization,searchDatasets, andckan.getOrgActivityStreamare all awaited with no try/catch. A CKAN backend hiccup on any of these will surface as an unhandled 500 for the whole SSR request instead of a controlled error/fallback response.🤖 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 `@pages/`[org]/index.tsx around lines 18 - 58, The server-side data loading in getServerSideProps is missing error handling around the external CKAN calls, so failures from getOrganization, searchDatasets, or ckan.getOrgActivityStream can crash the SSR request. Wrap these awaits in try/catch inside getServerSideProps, use a safe fallback like notFound or an empty/default props response when CKAN is unavailable, and keep the existing orgName validation and org.activity_stream assignment flow intact.components/home/heroSectionLight/SearchForm.tsx-9-10 (1)
9-10: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard
theme.styleshere
themes/default/index.tsxdoesn't providestyles, so the default theme path will hitstyles.shadowMd,styles.bgDark, andstyles.textLightwithstylesundefined and crash before the form renders.🤖 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 `@components/home/heroSectionLight/SearchForm.tsx` around lines 9 - 10, The SearchForm component assumes theme.styles is always present, but the default theme path leaves it undefined and causes a crash before render. Update the destructuring in SearchForm to safely guard styles from useTheme() before reading shadowMd, bgDark, and textLight, and provide a fallback or conditional path when styles is missing so the form can render with the default theme.components/home/actions/actionCard.tsx-6-10 (1)
6-10: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard
theme.stylesincomponents/home/actions/actionCard.tsx.DefaultThemedoesn’t setstyles, sotheme.styles.shadowMdthrows when the default theme is active. Usetheme.styles?.shadowMd ?? ""here.🤖 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 `@components/home/actions/actionCard.tsx` around lines 6 - 10, The ActionCard component is reading theme.styles.shadowMd directly, but DefaultTheme may not define styles and can throw when the default theme is active. Update the Link className in ActionCard to safely access the theme object using theme.styles?.shadowMd with a fallback empty string, so the component renders correctly regardless of theme shape.components/home/mainSection/PopularDatasets.tsx-27-31 (1)
27-31: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard against missing
dataset.organizationbefore building the link.
dataset.organization.nameis accessed directly, butpages/[org]/[dataset]/index.tsxexplicitly checks!dataset.organizationbefore use, confirming this field can be missing. If any highlighted dataset lacks an organization, this will throw during SSR of the home page.🐛 Proposed fix
- href={`/@${dataset.organization.name}/${dataset.name}`} + href={dataset.organization ? `/@${dataset.organization.name}/${dataset.name}` : "#"}🤖 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 `@components/home/mainSection/PopularDatasets.tsx` around lines 27 - 31, The PopularDatasets map is reading dataset.organization.name directly while building the Link href, but dataset.organization can be missing and cause SSR crashes. Update PopularDatasets to guard against a missing organization before constructing the href, either by skipping that item or rendering a safe fallback, and keep the check aligned with the existing dataset.organization handling used in the page component.pages/search.tsx-17-41 (1)
17-41: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSearch page ignores URL query params on initial SSR load.
The
TODOon Line 18 confirmsinitialRequestOptionis hardcoded and doesn't readq/filters from the request URL. This means direct navigation to a shared/filtered search link (e.g./search?q=climate&tags=...) will SSR the unfiltered default result set, and the client will have to refetch with the real filters after hydration — causing a visible flash/flicker of wrong results and defeating the purpose of the SWRfallbackcache for that request.Do you want me to draft a fix that reads
context.query/context.resolvedUrlingetServerSidePropsto buildinitialRequestOption?🤖 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 `@pages/search.tsx` around lines 17 - 41, The SSR search bootstrap in getServerSideProps is hardcoded and ignores the URL query params, so initial results don’t match shared or filtered search links. Update getServerSideProps in pages/search.tsx to accept the request context and build initialRequestOption from context.query or context.resolvedUrl instead of static defaults. Ensure the parsed q/tags/groups/orgs/resFormat values are passed into searchDatasets and used for the unstable_serialize fallback key so the initial render matches the requested filters.components/responsiveGrid/Pagination.tsx-18-33 (1)
18-33: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPagination controls aren't keyboard accessible.
<a>elements withouthrefaren't focusable/operable by keyboard by default, and there's noaria-disabled/visual disabled state when on the first/last page. Given this PR adds a Pa11y/Lighthouse accessibility pipeline, this pattern will likely fail those checks.♿ Proposed fix
- <a - onClick={() => setCurrentPage((prev) => Math.max(prev - 1, 1))} - className="cursor-pointer relative inline-flex items-center rounded-md bg-white px-3 py-2 text-sm font-semibold text-gray-900 ring-1 ring-inset ring-gray-300 hover:bg-gray-50 focus-visible:outline-offset-0" - aria-label="Previous page" - > - Previous - </a> + <button + type="button" + disabled={currentPage <= 1} + onClick={() => setCurrentPage((prev) => Math.max(prev - 1, 1))} + className="relative inline-flex items-center rounded-md bg-white px-3 py-2 text-sm font-semibold text-gray-900 ring-1 ring-inset ring-gray-300 hover:bg-gray-50 focus-visible:outline-offset-0 disabled:opacity-50 disabled:cursor-not-allowed" + aria-label="Previous page" + > + Previous + </button>🤖 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 `@components/responsiveGrid/Pagination.tsx` around lines 18 - 33, Pagination controls in Pagination.tsx use anchor tags without href, so they are not keyboard accessible and lack proper disabled behavior. Update the Previous/Next controls in the Pagination component to use semantic buttons (or add proper href/keyboard handling), and wire in disabled/aria-disabled states when currentPage is at the first or last page. Keep the existing setCurrentPage logic, but ensure the clickable controls are focusable and operable via keyboard in a way that passes the accessibility checks.pages/api/search-resource-data.tsx-29-36 (1)
29-36: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMissing null-guard on cell values causes 500 on blank CSV cells.
columnValue.toString()will throw if a cell isnull/undefined(common for blank CSV values), turning a normal empty cell into a 500 error for the whole search request. The client-side equivalent inDataProvider.tsxguards this withvalue?.toString().🐛 Proposed fix
const matchingRows = rows.filter((row) => Object.values(row).some((columnValue) => - columnValue + columnValue + ?.toString() - .toString() .toLowerCase() .includes((query as string).toLowerCase()) ) );🤖 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 `@pages/api/search-resource-data.tsx` around lines 29 - 36, The search filter in the rows matching logic can throw on blank CSV cells because `columnValue.toString()` assumes every cell is non-null. Update the `matchingRows` filter in `search-resource-data.tsx` to null-guard each `columnValue` before converting it to a string, matching the safe handling used in `DataProvider.tsx` so null or undefined values are treated as non-matches instead of causing a 500.components/responsiveGrid/SearchDataForm.tsx-10-19 (1)
10-19: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUnescaped query params and unhandled fetch errors.
valueanddataUrlare interpolated directly into the URL withoutencodeURIComponent; special characters (e.g.&,#,%) in either will corrupt the query string or unintentionally override theurlparam. Additionally, a fetch failure throws inside an async function invoked from a baresetTimeoutcallback, producing an unhandled rejection with no user-visible feedback.🐛 Proposed fix
const queryData = async (value) => { - const response = await fetch( - `/api/search-resource-data?query=${value}&url=${dataUrl}` - ); - if (!response.ok) { - throw new Error(`Failed to search data`); - } - const filteredData = await response.json(); - setTableData(Papa.unparse(filteredData)); + try { + const response = await fetch( + `/api/search-resource-data?query=${encodeURIComponent(value)}&url=${encodeURIComponent(dataUrl)}` + ); + if (!response.ok) { + throw new Error(`Failed to search data`); + } + const filteredData = await response.json(); + setTableData(Papa.unparse(filteredData)); + } catch (err) { + console.error(err); + } };🤖 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 `@components/responsiveGrid/SearchDataForm.tsx` around lines 10 - 19, The query building and error handling in SearchDataForm’s queryData need to be hardened. In queryData, encode both value and dataUrl before interpolating them into the /api/search-resource-data URL so special characters do not break or override query params. Also wrap the fetch/response handling in try/catch (or otherwise handle promise rejection) and surface a user-visible error state instead of letting the async error escape from the setTimeout-driven call. Use the queryData function, setTableData, and the fetch call in SearchDataForm as the main points to update.pages/api/search-resource-data.tsx-18-22 (1)
18-22: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNo timeout on outbound fetch.
An unresponsive or slow
urlhost will hang this request indefinitely, tying up the serverless/API request thread with no circuit breaker.⏱️ Proposed fix
- const response = await fetch(url as string); + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 10000); + const response = await fetch(url as string, { signal: controller.signal }); + clearTimeout(timeout);🤖 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 `@pages/api/search-resource-data.tsx` around lines 18 - 22, The fetch in the search-resource-data API route has no timeout, so a slow or unresponsive URL can hang the request indefinitely. Update the outbound request in the fetch flow to use an AbortController or equivalent timeout mechanism, and make sure the timeout error is handled cleanly alongside the existing non-OK response handling in the same response check logic.components/responsiveGrid/DataProvider.tsx-124-143 (1)
124-143: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUnhandled rejection & no error feedback on fetch failure.
fetchData's catch re-throws with no caller catching it (called bare insideuseEffect), producing an unhandled promise rejection. ThesetError/setLoadinghandling is commented out, so the UI has no failure state at all when the CSV fails to load.🐛 Proposed fix
+ const [error, setError] = useState<string | null>(null); + const fetchData = async () => { try { + setError(null); const response = await fetch(dataUrl); if (!response.ok) { throw new Error("Network response was not ok"); } const csvText = await response.text(); const parsedData = parseData(csvText); setData(parsedData.data); setVisibleColumns(Object.keys(parsedData.data[0] || {})); } catch (err) { - throw new Error(err.message); - //setError(err.message); // Handle errors (e.g., network issues) - // setLoading(false); + setError(err.message); } };🤖 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 `@components/responsiveGrid/DataProvider.tsx` around lines 124 - 143, In fetchData inside DataProvider, the catch block currently rethrows an error that useEffect does not catch, causing an unhandled promise rejection, and the commented-out setError/setLoading leaves no visible failure state. Update fetchData to handle failures locally by calling setError and always clearing loading state, and make the useEffect invocation of fetchData safely handle the async call without letting the rejection escape.components/responsiveGrid/DataProvider.tsx-160-217 (1)
160-217: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
filteredDatarecomputed unmemoized with a debugconsole.log, andcurrentPagenever resets on filter change.Two issues:
- Unlike
sortedData,filteredDataisn't wrapped inuseMemo, so it recalculates on every render, and line 165'sconsole.log(value)runs for every value of every row on every render — expensive and noisy for larger CSVs.currentPageisn't reset whenfilters/globalFilter/datachange, so filtering to a smaller result set can leave the user on an out-of-range page showing an empty table.🔧 Proposed fix
- const filteredData = sortedData.filter((row) => { + const filteredData = React.useMemo(() => sortedData.filter((row) => { // Apply global filter if ( globalFilter && !Object.values(row).some((value) => { - console.log(value); return value ?.toString() .toLowerCase() .includes(globalFilter.toLowerCase()); }) ) { return false; } ... return true; }); - }); + }), [sortedData, globalFilter, filters]); + + useEffect(() => { + setCurrentPage(1); + }, [globalFilter, filters, data]);🤖 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 `@components/responsiveGrid/DataProvider.tsx` around lines 160 - 217, The filtered dataset logic in DataProvider is doing unnecessary work on every render and leaves pagination stale after filter changes. Move the filteredData computation into useMemo alongside sortedData, remove the debug console.log from the row scan, and make the memo depend on sortedData, filters, and globalFilter. Also reset currentPage when filters/globalFilter/data change so pagination stays valid after the result set shrinks.components/responsiveGrid/Table.tsx-20-23 (1)
20-23: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winARIA role mismatch:
role="gridcell"used insiderole="table", notrole="grid".Per the ARIA spec,
gridcellmust be a direct descendant of one of the following roles: row which itself must belong to agrid/treegridcontainer — not a plaintable. Hererole="table"(Line 21) is combined downstream withrole="gridcell"inTableColValue.tsxandrole="columnheader"inTableHeadCell.tsx, producing an invalid ARIA structure that will confuse assistive technology. Either promote the container to a properrole="grid"(with roving tabindex per the MDN grid example) or drop the custom roles and rely on native<table>/<tr>/<td>semantics (which already imply table/row/cell).🤖 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 `@components/responsiveGrid/Table.tsx` around lines 20 - 23, The table container uses a plain table role while descendant cells are assigned grid-specific ARIA roles, creating an invalid accessibility hierarchy. Update Table and the related cell/header components (TableColValue and TableHeadCell) so the structure is consistent: either change the container to a proper grid/treegrid pattern with matching row and gridcell/columnheader roles, or remove the custom ARIA roles and rely on native table semantics throughout.components/responsiveGrid/SettingsDisplay.tsx-37-37 (1)
37-37: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winColumn list built from
data[0]keys instead of contextcolumns.
cols/filteredColsare derived fromObject.keys(data[0] || {}), while the header count (Columns ({columns.length}), Line 72) and "Check All" logic (Lines 40, 52, 56) use thecolumnsfield from context. If the first row doesn't contain every field present incolumns(sparse/inconsistent CSV rows are common), the panel will display a count that doesn't match the actual toggleable list, and some real columns will never be listed for toggling even though "Check All" would still mark them visible.Proposed fix
- const cols = Object.keys(data[0] || {}); + const cols = columns;Also applies to: 59-61, 72-72
🤖 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 `@components/responsiveGrid/SettingsDisplay.tsx` at line 37, The toggle list in SettingsDisplay is being built from data[0] keys instead of the context-provided columns, which can make the header count and “Check All” state inconsistent with the actual available fields. Update the cols/filteredCols logic in SettingsDisplay to derive from columns from context rather than Object.keys(data[0] || {}), and keep the existing check-all and count logic aligned with that same source so every real column can be toggled consistently.components/responsiveGrid/TableHeadCell.tsx-20-23 (1)
20-23: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUnbounded/unguarded
Math.min/Math.maxover the full dataset, computed redundantly.Several issues here:
- If
datahas anynull/undefinedvalues forkey,Math.min(...)/Math.max(...)will silently coercenullto0(wrong bound) or produceNaN(ifundefined), breaking the slider'smin/max.- Using spread (
Math.min(...data.map(...))) over very large arrays risks hitting the call-stack limit for large CSV datasets.- The same
data.map((row) => row[key])pass is recomputed 4 times (Lines 20, 21, 68, 69) on every render, unnecessarily re-scanning the whole dataset.Consider computing bounds once with a single reduce (filtering out non-numeric values) and memoizing with
useMemokeyed on[data, key].Proposed fix
- const min = Math.min(...data.map((row) => row[key])); - const max = Math.max(...data.map((row) => row[key])); + const { min, max } = useMemo(() => { + const nums = data + .map((row) => row[key]) + .filter((v) => typeof v === "number" && !Number.isNaN(v)); + return { + min: nums.length ? Math.min(...nums) : 0, + max: nums.length ? Math.max(...nums) : 0, + }; + }, [data, key]);Also applies to: 68-69
🤖 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 `@components/responsiveGrid/TableHeadCell.tsx` around lines 20 - 23, The slider bounds in TableHeadCell are being derived by repeatedly spreading the full data set into Math.min/Math.max, which is both unsafe for null/undefined values and inefficient for large arrays. Update the bound calculation used by TableHeadCell to compute min/max once with a single pass over data that filters out non-numeric row[key] values, and memoize the result with useMemo keyed on data and key. Then reuse those cached bounds wherever the component currently re-scans data (including the slider render logic) so the dataset is only traversed once per relevant change.components/responsiveGrid/TableActions.tsx-18-25 (1)
18-25: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
<a>withouthrefis not keyboard accessible.The export trigger is an anchor with only
onClickand nohref, so it won't be part of the natural tab order and can't be activated via Enter/Space by keyboard users. Use a<button>instead.Proposed fix
- <a - onClick={handleDownload} - className="cursor-pointer bg-accent hover:bg-accent-600 text-white transition-all inline-flex w-full justify-center gap-x-1.5 rounded-md px-4 py-2 text-sm font-semibold shadow-sm ring-1 ring-inset ring-accent " - > - Export - </a> + <button + type="button" + onClick={handleDownload} + className="cursor-pointer bg-accent hover:bg-accent-600 text-white transition-all inline-flex w-full justify-center gap-x-1.5 rounded-md px-4 py-2 text-sm font-semibold shadow-sm ring-1 ring-inset ring-accent " + > + Export + </button>🤖 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 `@components/responsiveGrid/TableActions.tsx` around lines 18 - 25, The Export trigger in TableActions is using an anchor without an href, which makes it inaccessible by keyboard. Update the clickable element in the TableActions component to use a button-style control instead of the current <a> with handleDownload, keeping the same styling and click behavior while ensuring it participates in the tab order and supports keyboard activation.
| const { url, query } = req.query; | ||
|
|
||
| if (!url) { | ||
| res.status(400).json({ error: "Missing 'url' query parameter." }); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| // Fetch the CSV file from the provided URL | ||
| const response = await fetch(url as string); | ||
| if (!response.ok) { | ||
| throw new Error(`Failed to fetch CSV file: ${response.statusText}`); | ||
| } | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files 'pages/api/search-resource-data.tsx' 'pages/**' | sed -n '1,120p'
echo '--- search-resource-data.tsx outline ---'
ast-grep outline pages/api/search-resource-data.tsx --view expanded || true
echo '--- search-resource-data.tsx contents ---'
wc -l pages/api/search-resource-data.tsx
cat -n pages/api/search-resource-data.tsx | sed -n '1,220p'
echo '--- find related CSV parsing code ---'
rg -n "Papa|parse\\(|toString\\(\\)|NEXT_PUBLIC_DMS|search-resource-data|DataProvider" pages -SRepository: datopian/portal-eiti
Length of output: 3386
Restrict url before fetching
url comes straight from the request and is fetched server-side, so this route can be used to probe internal hosts or metadata endpoints. Validate the origin against NEXT_PUBLIC_DMS before calling fetch.
🤖 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 `@pages/api/search-resource-data.tsx` around lines 9 - 22, The search-resource
API currently fetches the request’s url directly, which allows server-side
requests to arbitrary hosts. In the route handler in search-resource-data,
validate the supplied url against NEXT_PUBLIC_DMS before calling fetch, and
reject anything whose origin does not match the allowed DMS origin. Keep the
existing missing-url guard, but add the origin check immediately before the
fetch call so only approved resource URLs are retrieved.
|
Reimplemented on #5 |
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Chores