Skip to content
Open
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import type { Meta, StoryObj } from "@storybook/react";
import { buildApiUrl } from "@tests/utils/handlers";
import { HttpResponse, http } from "msw";
import { useState } from "react";
import { createFakeBlockDocument } from "@/mocks";
import { reactQueryDecorator } from "@/storybook/utils";
import { BlockDocumentCombobox } from "./block-document-combobox";

const MOCK_BLOCK_DOCUMENTS_DATA = Array.from({ length: 5 }, (_, i) =>
createFakeBlockDocument({ name: `my-block-${i}` }),
);

const meta = {
title: "Components/Blocks/BlockDocumentCombobox",
render: (args) => <BlockDocumentComboboxStory {...args} />,
decorators: [reactQueryDecorator],
parameters: {
msw: {
handlers: [
http.post(buildApiUrl("/block_documents/filter"), () => {
return HttpResponse.json(MOCK_BLOCK_DOCUMENTS_DATA);
}),
],
},
},
args: {
blockTypeSlug: "aws-credentials",
},
} satisfies Meta<{ blockTypeSlug: string; showCreateNew?: boolean }>;

export default meta;

type Story = StoryObj<typeof meta>;

export const Default: Story = { name: "BlockDocumentCombobox" };

export const WithCreateNew: Story = {
name: "With Create New Button",
args: {
showCreateNew: true,
},
};

export const Empty: Story = {
name: "Empty State",
parameters: {
msw: {
handlers: [
http.post(buildApiUrl("/block_documents/filter"), () => {
return HttpResponse.json([]);
}),
],
},
},
};

const BlockDocumentComboboxStory = ({
blockTypeSlug,
showCreateNew,
}: {
blockTypeSlug: string;
showCreateNew?: boolean;
}) => {
const [selected, setSelected] = useState<string | undefined>();

return (
<BlockDocumentCombobox
blockTypeSlug={blockTypeSlug}
selectedBlockDocumentId={selected}
onSelect={setSelected}
onCreateNew={
showCreateNew ? () => alert("Create new clicked") : undefined
}
/>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import { render, screen, waitFor } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { buildApiUrl, createWrapper, server } from "@tests/utils";
import { mockPointerEvents } from "@tests/utils/browser";
import { HttpResponse, http } from "msw";
import { beforeAll, describe, expect, it, vi } from "vitest";
import type { components } from "@/api/prefect";
import { createFakeBlockDocument } from "@/mocks";
import { BlockDocumentCombobox } from "./block-document-combobox";

describe("BlockDocumentCombobox", () => {
beforeAll(mockPointerEvents);

const mockListBlockDocumentsAPI = (
blockDocuments: Array<components["schemas"]["BlockDocument"]>,
) => {
server.use(
http.post(buildApiUrl("/block_documents/filter"), () => {
return HttpResponse.json(blockDocuments);
}),
);
};

it("able to select a block document", async () => {
const mockOnSelect = vi.fn();
const blockDocuments = [
createFakeBlockDocument({ id: "block-1", name: "my_block_0" }),
createFakeBlockDocument({ id: "block-2", name: "my_block_1" }),
];
mockListBlockDocumentsAPI(blockDocuments);

const user = userEvent.setup();

render(
<BlockDocumentCombobox
blockTypeSlug="aws-credentials"
selectedBlockDocumentId={undefined}
onSelect={mockOnSelect}
/>,
{ wrapper: createWrapper() },
);

await waitFor(() =>
expect(screen.getByLabelText(/select a block/i)).toBeVisible(),
);

await user.click(screen.getByLabelText(/select a block/i));
await user.click(screen.getByRole("option", { name: "my_block_0" }));

expect(mockOnSelect).toHaveBeenLastCalledWith("block-1");
});

it("has the selected value displayed", async () => {
const blockDocuments = [
createFakeBlockDocument({ id: "block-1", name: "my_block_0" }),
createFakeBlockDocument({ id: "block-2", name: "my_block_1" }),
];
mockListBlockDocumentsAPI(blockDocuments);

render(
<BlockDocumentCombobox
blockTypeSlug="aws-credentials"
selectedBlockDocumentId="block-1"
onSelect={vi.fn()}
/>,
{ wrapper: createWrapper() },
);

await waitFor(() => expect(screen.getByText("my_block_0")).toBeVisible());
});

it("shows placeholder when no block document is selected", async () => {
mockListBlockDocumentsAPI([]);

render(
<BlockDocumentCombobox
blockTypeSlug="aws-credentials"
selectedBlockDocumentId={undefined}
onSelect={vi.fn()}
/>,
{ wrapper: createWrapper() },
);

await waitFor(() =>
expect(screen.getByText("Select a block...")).toBeVisible(),
);
});

it("shows create new button when onCreateNew is provided", async () => {
const mockOnCreateNew = vi.fn();
mockListBlockDocumentsAPI([]);

const user = userEvent.setup();

render(
<BlockDocumentCombobox
blockTypeSlug="aws-credentials"
selectedBlockDocumentId={undefined}
onSelect={vi.fn()}
onCreateNew={mockOnCreateNew}
/>,
{ wrapper: createWrapper() },
);

await waitFor(() =>
expect(screen.getByLabelText(/select a block/i)).toBeVisible(),
);

await user.click(screen.getByLabelText(/select a block/i));
await user.click(screen.getByRole("option", { name: /create new block/i }));

expect(mockOnCreateNew).toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
import { useSuspenseQuery } from "@tanstack/react-query";
import { Suspense, useDeferredValue, useMemo, useState } from "react";
import { buildListFilterBlockDocumentsQuery } from "@/api/block-documents";
import {
Combobox,
ComboboxCommandEmtpy,
ComboboxCommandGroup,
ComboboxCommandInput,
ComboboxCommandItem,
ComboboxCommandList,
ComboboxContent,
ComboboxTrigger,
} from "@/components/ui/combobox";
import { Icon } from "@/components/ui/icons";

type BlockDocumentComboboxProps = {
blockTypeSlug: string;
selectedBlockDocumentId: string | undefined;
onSelect: (blockDocumentId: string | undefined) => void;
onCreateNew?: () => void;
};

export const BlockDocumentCombobox = ({
blockTypeSlug,
selectedBlockDocumentId,
onSelect,
onCreateNew,
}: BlockDocumentComboboxProps) => {
return (
<Suspense>
<BlockDocumentComboboxImplementation
blockTypeSlug={blockTypeSlug}
selectedBlockDocumentId={selectedBlockDocumentId}
onSelect={onSelect}
onCreateNew={onCreateNew}
/>
</Suspense>
);
Comment on lines +29 to +38

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Missing Suspense fallback leaves users with no loading indication.

The <Suspense> component has no fallback prop, so users won't see any visual feedback while the block documents are loading. Consider adding a skeleton or loading indicator.

🛡️ Suggested fix
 return (
-  <Suspense>
+  <Suspense fallback={<ComboboxTrigger disabled>Loading...</ComboboxTrigger>}>
     <BlockDocumentComboboxImplementation
🤖 Prompt for AI Agents
In
`@ui-v2/src/components/blocks/block-document-combobox/block-document-combobox.tsx`
around lines 29 - 38, The Suspense wrapper in BlockDocumentCombobox currently
has no fallback, so add a meaningful fallback prop (e.g., a skeleton or spinner)
to the Suspense that renders while BlockDocumentComboboxImplementation is
loading; update the <Suspense> around BlockDocumentComboboxImplementation to
include a fallback component (a small loader/skeleton component you create or an
existing Spinner) to provide visual loading feedback.

};

const BlockDocumentComboboxImplementation = ({
blockTypeSlug,
selectedBlockDocumentId,
onSelect,
onCreateNew,
}: BlockDocumentComboboxProps) => {
const [search, setSearch] = useState("");
const deferredSearch = useDeferredValue(search);

const { data } = useSuspenseQuery(
buildListFilterBlockDocumentsQuery({
offset: 0,
sort: "BLOCK_TYPE_AND_NAME_ASC",
include_secrets: false,
block_types: {
slug: { any_: [blockTypeSlug] },
},
block_documents: {
operator: "and_",
is_anonymous: { eq_: false },
...(deferredSearch ? { name: { like_: deferredSearch } } : {}),
},
limit: 50,
}),
);

const filteredData = useMemo(() => {
return data.filter((blockDocument) =>
blockDocument.name?.toLowerCase().includes(deferredSearch.toLowerCase()),
);
}, [data, deferredSearch]);

const selectedBlockDocument = useMemo(() => {
return filteredData.find(
(blockDocument) => blockDocument.id === selectedBlockDocumentId,
);
}, [filteredData, selectedBlockDocumentId]);
Comment on lines +67 to +77

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Client-side filtering may be redundant and selectedBlockDocument lookup could fail.

Two concerns:

  1. Redundant filtering: The server-side query already filters by name: { like_: deferredSearch } (Line 61), so the client-side filteredData filter appears redundant unless the API filter behaves differently than expected.

  2. Selected document may disappear: selectedBlockDocument is looked up in filteredData rather than data. If the user has selected a document and then types a search term that doesn't match, selectedBlockDocument becomes undefined, and the trigger will show "Select a block..." even though a block is actually selected.

🐛 Suggested fix for the selected document lookup
-const selectedBlockDocument = useMemo(() => {
-  return filteredData.find(
-    (blockDocument) => blockDocument.id === selectedBlockDocumentId,
-  );
-}, [filteredData, selectedBlockDocumentId]);
+const selectedBlockDocument = useMemo(() => {
+  return data.find(
+    (blockDocument) => blockDocument.id === selectedBlockDocumentId,
+  );
+}, [data, selectedBlockDocumentId]);
📝 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.

Suggested change
const filteredData = useMemo(() => {
return data.filter((blockDocument) =>
blockDocument.name?.toLowerCase().includes(deferredSearch.toLowerCase()),
);
}, [data, deferredSearch]);
const selectedBlockDocument = useMemo(() => {
return filteredData.find(
(blockDocument) => blockDocument.id === selectedBlockDocumentId,
);
}, [filteredData, selectedBlockDocumentId]);
const filteredData = useMemo(() => {
return data.filter((blockDocument) =>
blockDocument.name?.toLowerCase().includes(deferredSearch.toLowerCase()),
);
}, [data, deferredSearch]);
const selectedBlockDocument = useMemo(() => {
return data.find(
(blockDocument) => blockDocument.id === selectedBlockDocumentId,
);
}, [data, selectedBlockDocumentId]);
🤖 Prompt for AI Agents
In
`@ui-v2/src/components/blocks/block-document-combobox/block-document-combobox.tsx`
around lines 67 - 77, filteredData duplicates server-side filtering and
selectedBlockDocument is incorrectly derived from filteredData which makes a
previously selected item vanish when the search excludes it; change
selectedBlockDocument to search the full data array (use data.find by
selectedBlockDocumentId) and either remove the client-side filtering
(filteredData) or keep it only for listing results (but never use it to resolve
the selected item), ensuring references to filteredData, data, deferredSearch,
and selectedBlockDocumentId are updated accordingly.


return (
<Combobox>
<ComboboxTrigger
selected={Boolean(selectedBlockDocumentId)}
aria-label="Select a block"
>
{selectedBlockDocument?.name ?? "Select a block..."}
</ComboboxTrigger>
<ComboboxContent>
<ComboboxCommandInput
value={search}
onValueChange={setSearch}
placeholder="Search for a block..."
/>
<ComboboxCommandEmtpy>No block found</ComboboxCommandEmtpy>
<ComboboxCommandList>
<ComboboxCommandGroup>
{filteredData.map((blockDocument) => (
<ComboboxCommandItem
key={blockDocument.id}
selected={selectedBlockDocumentId === blockDocument.id}
onSelect={(value) => {
onSelect(value);
setSearch("");
}}
value={blockDocument.id}
>
{blockDocument.name}
</ComboboxCommandItem>
))}
</ComboboxCommandGroup>
{onCreateNew && (
<ComboboxCommandGroup>
<ComboboxCommandItem
onSelect={() => {
onCreateNew();
setSearch("");
}}
value="__create_new__"
closeOnSelect={true}
>
<Icon id="Plus" className="mr-2 size-4" />
Create new block
</ComboboxCommandItem>
</ComboboxCommandGroup>
)}
</ComboboxCommandList>
</ComboboxContent>
</Combobox>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { BlockDocumentCombobox } from "./block-document-combobox";
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import type { Meta, StoryObj } from "@storybook/react";
import { buildApiUrl } from "@tests/utils/handlers";
import { HttpResponse, http } from "msw";
import { useState } from "react";
import { Button } from "@/components/ui/button";
import { createFakeBlockSchema, createFakeBlockType } from "@/mocks";
import { reactQueryDecorator, toastDecorator } from "@/storybook/utils";
import { BlockDocumentCreateDialog } from "./block-document-create-dialog";

const MOCK_BLOCK_TYPE = createFakeBlockType({
id: "block-type-1",
slug: "secret",
name: "Secret",
});

const MOCK_BLOCK_SCHEMA = createFakeBlockSchema();

const meta = {
title: "Components/Blocks/BlockDocumentCreateDialog",
render: (args) => <BlockDocumentCreateDialogStory {...args} />,
decorators: [reactQueryDecorator, toastDecorator],
parameters: {
msw: {
handlers: [
http.get(buildApiUrl("/block_types/slug/:slug"), () => {
return HttpResponse.json(MOCK_BLOCK_TYPE);
}),
http.post(buildApiUrl("/block_schemas/filter"), () => {
return HttpResponse.json([
{ ...MOCK_BLOCK_SCHEMA, block_type_id: MOCK_BLOCK_TYPE.id },
]);
}),
http.post(buildApiUrl("/block_documents/"), () => {
return HttpResponse.json({
id: "new-block-document-id",
name: "test-block",
});
}),
],
},
},
args: {
blockTypeSlug: "secret",
},
} satisfies Meta<{ blockTypeSlug: string }>;

export default meta;

type Story = StoryObj<typeof meta>;

export const Default: Story = { name: "BlockDocumentCreateDialog" };

const BlockDocumentCreateDialogStory = ({
blockTypeSlug,
}: {
blockTypeSlug: string;
}) => {
const [open, setOpen] = useState(false);

return (
<>
<Button onClick={() => setOpen(true)}>Open Dialog</Button>
<BlockDocumentCreateDialog
open={open}
onOpenChange={setOpen}
blockTypeSlug={blockTypeSlug}
onCreated={(id) => alert(`Created block document: ${id}`)}
/>
</>
);
};
Loading
Loading