Skip to content

fix: resolve search ranking showing N/A on admin page #256

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
snomiao wants to merge 7 commits into
base: main
Choose a base branch
from
Open

fix: resolve search ranking showing N/A on admin page #256

Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4dedb0f
fix: resolve search ranking showing N/A by fetching individual node data
snomiao Mar 31, 2026
576e3d5
format: Apply prettier --fix changes
snomiao Mar 31, 2026
c2a079b
Update pages/admin/search-ranking.tsx
snomiao Apr 4, 2026
f54b797
ci: run Next.js CI on all pull requests (#255)
snomiao Apr 10, 2026
cf1086c
Merge branch 'fix/admin-search-ranking-na' of https://github.com/Comf…
snomiao Apr 10, 2026
7e31cd5
fix: pass query options correctly to useGetNode and handle error state
snomiao Apr 10, 2026
b86c9cb
format: Apply prettier --fix changes
snomiao Apr 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions pages/admin/search-ranking.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,19 @@ import { CustomPagination } from "@/components/common/CustomPagination";
import withAdmin from "@/components/common/HOC/authAdmin";
import { formatDownloadCount } from "@/components/nodes/NodeDetails";
import SearchRankingEditModal from "@/components/nodes/SearchRankingEditModal";
import { Node, useSearchNodes } from "@/src/api/generated";
import { Node, useGetNode, useSearchNodes } from "@/src/api/generated";
import { useNextTranslation } from "@/src/hooks/i18n";

function NodeSearchRankingCell({ nodeId }: { nodeId: string }) {
const { t } = useNextTranslation();
const { data: node, isLoading } = useGetNode(nodeId, {
staleTime: 5 * 60 * 1000,
refetchOnWindowFocus: false,

Copilot AI Apr 4, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useGetNode's second argument is params (querystring), not React Query options. Passing { staleTime, refetchOnWindowFocus } here will be sent as request params and the caching options won't apply. Move these under the 3rd argument as options: { query: { staleTime, refetchOnWindowFocus } } (and keep params as undefined unless needed).

Suggested change
const { data: node, isLoading } = useGetNode(nodeId, {
staleTime: 5 * 60 * 1000,
refetchOnWindowFocus: false,
const { data: node, isLoading } = useGetNode(nodeId, undefined, {
query: {
staleTime: 5 * 60 * 1000,
refetchOnWindowFocus: false,
},

Copilot uses AI. Check for mistakes.

@snomiao snomiao Apr 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7e31cd5. Moved staleTime and refetchOnWindowFocus to the 3rd argument under options.query, and passed undefined for params. Also added enabled: !!nodeId to prevent queries with empty strings.

});
if (isLoading) return <Spinner size="xs" />;

Copilot AI Apr 4, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cell treats any useGetNode failure the same as “no ranking” (it will render N/A whenever the request errors, since node is undefined and isLoading is false). Consider handling isError/error separately (e.g., render an explicit error state or retry affordance) so network/auth issues aren’t silently masked as missing data.

Suggested change
const { data: node, isLoading } = useGetNode(nodeId, {
staleTime: 5 * 60 * 1000,
refetchOnWindowFocus: false,
});
if (isLoading) return <Spinner size="xs" />;
const { data: node, isLoading, isError } = useGetNode(nodeId, {
staleTime: 5 * 60 * 1000,
refetchOnWindowFocus: false,
});
if (isLoading) return <Spinner size="xs" />;
if (isError) return <>Error</>;

Copilot uses AI. Check for mistakes.

@snomiao snomiao Apr 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7e31cd5. Added isError handling that shows an explicit "Error" state instead of silently displaying N/A.

return <>{node?.search_ranking != null ? node.search_ranking : t("N/A")}</>;
}

function SearchRankingAdminPage() {
const { t } = useNextTranslation();
const router = useRouter();
Expand Down Expand Up @@ -140,7 +150,7 @@ function SearchRankingAdminPage() {
<div className="truncate text-gray-300">{node.publisher?.id || t("N/A")}</div>
<div className="text-gray-300">{formatDownloadCount(node.downloads || 0)}</div>
<div className="text-gray-300">
{node.search_ranking !== undefined ? node.search_ranking : t("N/A")}
<NodeSearchRankingCell nodeId={node.id || ""} />

Copilot AI Apr 4, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

node.id is optional in the generated Node type; passing node.id || "" means the query will be disabled when id is missing and the cell will silently show N/A. Prefer guarding before rendering (e.g., only render the cell when node.id is truthy) so missing IDs are handled explicitly.

Copilot uses AI. Check for mistakes.

@snomiao snomiao Apr 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7e31cd5. Now guarding node.id before rendering NodeSearchRankingCell — renders t("N/A") directly when id is missing, avoiding the empty-string query key issue.

</div>

Copilot AI Apr 4, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rendering NodeSearchRankingCell for each row introduces an N+1 pattern (up to 24 extra GET /nodes/{id} requests per page load/search/pagination). This can significantly slow the admin page and increase API load; consider adding search_ranking to /nodes/search for admin responses, or introducing a batch endpoint / query that fetches rankings for the current page in one request.

Copilot uses AI. Check for mistakes.

@snomiao snomiao Apr 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged the N+1 concern. The ideal fix is to extend /nodes/search to include search_ranking for admin responses on the backend. This approach is a workaround since the frontend can't modify the API response. Added staleTime: 5min and refetchOnWindowFocus: false (now correctly placed in query options per 7e31cd5) to mitigate repeated refetches.

<div>
<Button size="xs" color="blue" onClick={() => handleEditRanking(node)}>
Expand Down
Loading