Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 16 additions & 5 deletions apps/desktop/src/lib/trpc/routers/projects/projects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,12 @@ export const createProjectsRouter = (getWindow: () => BrowserWindow | null) => {
}),

listPullRequests: publicProcedure
.input(z.object({ projectId: z.string() }))
.input(
z.object({
projectId: z.string(),
includeClosed: z.boolean().optional(),
}),
)
.query(async ({ input }) => {
const project = localDb
.select()
Expand All @@ -355,7 +360,7 @@ export const createProjectsRouter = (getWindow: () => BrowserWindow | null) => {
"pr",
"list",
"--state",
"open",
input.includeClosed ? "all" : "open",
"--limit",
"30",
"--json",
Expand All @@ -376,6 +381,7 @@ export const createProjectsRouter = (getWindow: () => BrowserWindow | null) => {
z.object({
projectId: z.string(),
query: z.string(),
includeClosed: z.boolean().optional(),
}),
)
.query(async ({ input }) => {
Expand All @@ -393,7 +399,7 @@ export const createProjectsRouter = (getWindow: () => BrowserWindow | null) => {
"pr",
"list",
"--state",
"all",
input.includeClosed ? "all" : "open",
"--search",
input.query,
"--limit",
Expand All @@ -412,7 +418,12 @@ export const createProjectsRouter = (getWindow: () => BrowserWindow | null) => {
}),

listIssues: publicProcedure
.input(z.object({ projectId: z.string() }))
.input(
z.object({
projectId: z.string(),
includeClosed: z.boolean().optional(),
}),
)
.query(async ({ input }) => {
const project = localDb
.select()
Expand All @@ -428,7 +439,7 @@ export const createProjectsRouter = (getWindow: () => BrowserWindow | null) => {
"issue",
"list",
"--state",
"open",
input.includeClosed ? "all" : "open",
"--limit",
"30",
"--json",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Checkbox } from "@superset/ui/checkbox";
import {
Command,
CommandEmpty,
Expand All @@ -11,7 +12,7 @@ import { Tooltip, TooltipContent, TooltipTrigger } from "@superset/ui/tooltip";
import { useLiveQuery } from "@tanstack/react-db";
import Fuse from "fuse.js";
import type { ReactNode } from "react";
import { useMemo, useState } from "react";
import { useId, useMemo, useState } from "react";
import {
StatusIcon,
type StatusType,
Expand All @@ -20,6 +21,10 @@ import { useCollections } from "renderer/routes/_authenticated/providers/Collect

const MAX_RESULTS = 20;

function isClosedStatus(type: StatusType | undefined): boolean {
return type === "completed" || type === "canceled";
}

interface IssueLinkCommandProps {
children: ReactNode;
tooltipLabel: string;
Expand All @@ -38,6 +43,8 @@ export function IssueLinkCommand({
}: IssueLinkCommandProps) {
const [open, setOpen] = useState(false);
const [searchQuery, setSearchQuery] = useState("");
const [showClosed, setShowClosed] = useState(false);
const showClosedId = useId();
const collections = useCollections();

const { data: allTasks } = useLiveQuery(
Expand Down Expand Up @@ -82,21 +89,35 @@ export function IssueLinkCommand({

const taskFuse = useMemo(
() =>
new Fuse(allTasks ?? [], {
keys: [
{ name: "slug", weight: 3 },
{ name: "title", weight: 2 },
],
threshold: 0.4,
ignoreLocation: true,
}),
[allTasks],
new Fuse(
(allTasks ?? []).filter((task) => {
if (showClosed) return true;
const status = task.statusId
? statusMap.get(task.statusId)
: undefined;
return !isClosedStatus(status?.type);
}),
{
keys: [
{ name: "slug", weight: 3 },
{ name: "title", weight: 2 },
],
threshold: 0.4,
ignoreLocation: true,
},
),
[allTasks, showClosed, statusMap],
);

const filteredTasks = useMemo(() => {
if (!allTasks?.length) return [];
const visibleTasks = allTasks.filter((task) => {
if (showClosed) return true;
Comment on lines +93 to +115
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Duplicate filter computation

The closed-status filter is computed in two places: once to build the Fuse index (lines 93–100) and again inside filteredTasks for the no-search-query branch (lines 110–115). When a search query is active the results come from taskFuse, which already excludes closed items, so the second filter is never needed for that path. Consider deriving a single visibleTasks array outside both useMemo calls and reusing it as the Fuse input.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/components/Chat/ChatInterface/components/IssueLinkCommand/IssueLinkCommand.tsx
Line: 93-115

Comment:
**Duplicate filter computation**

The closed-status filter is computed in two places: once to build the Fuse index (lines 93–100) and again inside `filteredTasks` for the no-search-query branch (lines 110–115). When a search query is active the results come from `taskFuse`, which already excludes closed items, so the second filter is never needed for that path. Consider deriving a single `visibleTasks` array outside both `useMemo` calls and reusing it as the Fuse input.

How can I resolve this? If you propose a fix, please make it concise.

const status = task.statusId ? statusMap.get(task.statusId) : undefined;
return !isClosedStatus(status?.type);
});
if (!searchQuery) {
return [...allTasks]
return visibleTasks
.sort(
(a, b) =>
new Date(b.updatedAt).getTime() - new Date(a.updatedAt).getTime(),
Expand All @@ -106,7 +127,7 @@ export function IssueLinkCommand({
return taskFuse
.search(searchQuery, { limit: MAX_RESULTS })
.map((r) => r.item);
}, [allTasks, searchQuery, taskFuse]);
}, [allTasks, searchQuery, showClosed, statusMap, taskFuse]);

const handleSelect = (
slug: string,
Expand Down Expand Up @@ -145,12 +166,35 @@ export function IssueLinkCommand({
value={searchQuery}
onValueChange={setSearchQuery}
/>
<div className="flex items-center gap-2 border-b px-3 py-2">
<Checkbox
id={showClosedId}
checked={showClosed}
onCheckedChange={(checked) => setShowClosed(checked === true)}
/>
<label
htmlFor={showClosedId}
className="cursor-pointer select-none text-xs text-muted-foreground"
>
Show closed
</label>
</div>
<CommandList className="max-h-[280px]">
{filteredTasks.length === 0 && (
<CommandEmpty>No issues found.</CommandEmpty>
<CommandEmpty>
{showClosed ? "No issues found." : "No open issues found."}
</CommandEmpty>
)}
{filteredTasks.length > 0 && (
<CommandGroup heading={searchQuery ? "Results" : "Recent issues"}>
<CommandGroup
heading={
searchQuery
? "Results"
: showClosed
? "Recent issues"
: "Open issues"
}
>
{filteredTasks.map((task) => {
const status = task.statusId
? statusMap.get(task.statusId)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Checkbox } from "@superset/ui/checkbox";
import {
Command,
CommandEmpty,
Expand All @@ -10,7 +11,7 @@ import { Popover, PopoverAnchor, PopoverContent } from "@superset/ui/popover";
import Fuse from "fuse.js";
import type React from "react";
import type { RefObject } from "react";
import { useMemo, useState } from "react";
import { useId, useMemo, useState } from "react";
import { electronTrpc } from "renderer/lib/electron-trpc";
import {
IssueIcon,
Expand Down Expand Up @@ -46,9 +47,11 @@ export function GitHubIssueLinkCommand({
anchorRef,
}: GitHubIssueLinkCommandProps) {
const [searchQuery, setSearchQuery] = useState("");
const [showClosed, setShowClosed] = useState(false);
const showClosedId = useId();

const { data: issues, isLoading } = electronTrpc.projects.listIssues.useQuery(
{ projectId: projectId ?? "" },
{ projectId: projectId ?? "", includeClosed: showClosed },
{ enabled: !!projectId && open },
);

Expand Down Expand Up @@ -122,14 +125,39 @@ export function GitHubIssueLinkCommand({
value={searchQuery}
onValueChange={setSearchQuery}
/>
<div className="flex items-center gap-2 border-b px-3 py-2">
<Checkbox
id={showClosedId}
checked={showClosed}
onCheckedChange={(checked) => setShowClosed(checked === true)}
/>
<label
htmlFor={showClosedId}
className="cursor-pointer select-none text-xs text-muted-foreground"
>
Show closed
</label>
</div>
<CommandList className="max-h-[280px]">
{searchResults.length === 0 && (
<CommandEmpty>
{isLoading ? "Loading issues..." : "No open issues found."}
{isLoading
? "Loading issues..."
: showClosed
? "No issues found."
: "No open issues found."}
</CommandEmpty>
)}
{searchResults.length > 0 && (
<CommandGroup heading={searchQuery ? "Results" : "Open issues"}>
<CommandGroup
heading={
searchQuery
? "Results"
: showClosed
? "Recent issues"
: "Open issues"
}
>
{searchResults.map((issue) => (
<CommandItem
key={issue.issueNumber}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Checkbox } from "@superset/ui/checkbox";
import {
Command,
CommandEmpty,
Expand All @@ -9,7 +10,7 @@ import {
import { Popover, PopoverAnchor, PopoverContent } from "@superset/ui/popover";
import type React from "react";
import type { RefObject } from "react";
import { useMemo, useState } from "react";
import { useId, useMemo, useState } from "react";
import { useDebouncedValue } from "renderer/hooks/useDebouncedValue";
import { electronTrpc } from "renderer/lib/electron-trpc";
import {
Expand Down Expand Up @@ -62,6 +63,8 @@ export function PRLinkCommand({
anchorRef,
}: PRLinkCommandProps) {
const [searchQuery, setSearchQuery] = useState("");
const [showClosed, setShowClosed] = useState(false);
const showClosedId = useId();
const debouncedQuery = useDebouncedValue(searchQuery, 300);
const trimmedQuery = searchQuery.trim(); // Immediate trim for UI decisions
const debouncedTrimmed = debouncedQuery.trim(); // Debounced trim for RPC calls
Expand Down Expand Up @@ -99,14 +102,18 @@ export function PRLinkCommand({
// Fetch recent PRs for browsing (only when no search query)
const { data: recentPRs, isLoading: isLoadingRecent } =
electronTrpc.projects.listPullRequests.useQuery(
{ projectId: projectId ?? "" },
{ projectId: projectId ?? "", includeClosed: showClosed },
{ enabled: !!projectId && open && !debouncedTrimmed },
);

// Server-side search when user types (use debounced for RPC)
const { data: searchResults, isLoading: isSearching } =
electronTrpc.projects.searchPullRequests.useQuery(
{ projectId: projectId ?? "", query: effectiveQuery },
{
projectId: projectId ?? "",
query: effectiveQuery,
includeClosed: showClosed,
},
{
enabled:
!!projectId && open && !!effectiveQuery && !isCrossRepositoryUrl,
Expand Down Expand Up @@ -164,6 +171,19 @@ export function PRLinkCommand({
value={searchQuery}
onValueChange={setSearchQuery}
/>
<div className="flex items-center gap-2 border-b px-3 py-2">
<Checkbox
id={showClosedId}
checked={showClosed}
onCheckedChange={(checked) => setShowClosed(checked === true)}
/>
<label
htmlFor={showClosedId}
className="cursor-pointer select-none text-xs text-muted-foreground"
>
Show closed
</label>
</div>
<CommandList className="max-h-[280px]">
{pullRequests.length === 0 && (
<CommandEmpty>
Expand All @@ -175,15 +195,19 @@ export function PRLinkCommand({
? `PR URL must match ${selectedRepositoryLabel}.`
: debouncedTrimmed
? "No pull requests found."
: "No open pull requests."}
: showClosed
? "No pull requests found."
: "No open pull requests."}
</CommandEmpty>
)}
{pullRequests.length > 0 && (
<CommandGroup
heading={
debouncedTrimmed
? `${pullRequests.length} result${pullRequests.length === 1 ? "" : "s"}`
: "Recent pull requests"
: showClosed
? "Recent pull requests"
: "Open pull requests"
}
>
{pullRequests.map((pr) => (
Expand Down
Loading
Loading