fix: address capability discovery review findings
This commit is contained in:
@@ -102,6 +102,30 @@ describe("DiscoverRoute", () => {
|
||||
expect(screen.queryByRole("button", { name: /add to draft/i })).toBeNull();
|
||||
});
|
||||
|
||||
it("exposes selected row state and associates the result with its detail", () => {
|
||||
mockedUseCapabilityDiscovery.mockReturnValue(
|
||||
controller({
|
||||
selected: {
|
||||
...summary,
|
||||
isAsync: false,
|
||||
inputSchema: {},
|
||||
outputSchema: {},
|
||||
wrapperHints: {},
|
||||
acceptsContext: true,
|
||||
},
|
||||
}),
|
||||
);
|
||||
render(<DiscoverRoute />);
|
||||
|
||||
const row = screen.getByRole("button", { name: /local\.documents\.read/i });
|
||||
expect(row).toHaveAttribute("aria-pressed", "true");
|
||||
expect(row).toHaveAttribute("aria-controls", "capability-detail");
|
||||
expect(screen.getByRole("region", { name: "local.documents.read" })).toHaveAttribute(
|
||||
"id",
|
||||
"capability-detail",
|
||||
);
|
||||
});
|
||||
|
||||
it("shows load more only when the controller has a next cursor", async () => {
|
||||
const loadMore = vi.fn();
|
||||
mockedUseCapabilityDiscovery.mockReturnValue(
|
||||
@@ -117,4 +141,13 @@ describe("DiscoverRoute", () => {
|
||||
render(<DiscoverRoute />);
|
||||
expect(screen.queryByRole("button", { name: "Load more capabilities" })).toBeNull();
|
||||
});
|
||||
|
||||
it("disables load more while the controller is loading", () => {
|
||||
mockedUseCapabilityDiscovery.mockReturnValue(
|
||||
controller({ phase: "loading", nextCursor: "page-2" }),
|
||||
);
|
||||
render(<DiscoverRoute />);
|
||||
|
||||
expect(screen.getByRole("button", { name: "Load more capabilities" })).toBeDisabled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -39,6 +39,8 @@ const CapabilityRow = ({
|
||||
<button
|
||||
className="capability-discovery__row"
|
||||
data-selected={selected}
|
||||
aria-controls="capability-detail"
|
||||
aria-pressed={selected}
|
||||
onClick={() => onInspect(item.name)}
|
||||
type="button"
|
||||
>
|
||||
@@ -61,7 +63,7 @@ const CapabilityRow = ({
|
||||
);
|
||||
|
||||
const DetailView = ({ detail }: { readonly detail: CapabilityDetail }) => (
|
||||
<section aria-labelledby="capability-detail-heading" className="capability-discovery__detail">
|
||||
<section aria-labelledby="capability-detail-heading" className="capability-discovery__detail" id="capability-detail">
|
||||
<p className="workspace-route-pending__eyebrow">Selected contract</p>
|
||||
<h2 id="capability-detail-heading">{detail.name}</h2>
|
||||
<dl className="capability-discovery__facts">
|
||||
@@ -151,7 +153,11 @@ export const DiscoverRoute = () => {
|
||||
</ul>
|
||||
)}
|
||||
{discovery.nextCursor && (
|
||||
<button onClick={discovery.loadMore} type="button">
|
||||
<button
|
||||
disabled={discovery.phase === "loading"}
|
||||
onClick={discovery.loadMore}
|
||||
type="button"
|
||||
>
|
||||
Load more capabilities
|
||||
</button>
|
||||
)}
|
||||
@@ -160,7 +166,7 @@ export const DiscoverRoute = () => {
|
||||
{discovery.selected ? (
|
||||
<DetailView detail={discovery.selected} />
|
||||
) : (
|
||||
<section aria-labelledby="capability-detail-empty-heading" className="capability-discovery__detail capability-discovery__detail--empty">
|
||||
<section aria-labelledby="capability-detail-empty-heading" className="capability-discovery__detail capability-discovery__detail--empty" id="capability-detail">
|
||||
<p className="workspace-route-pending__eyebrow">Contract detail</p>
|
||||
<h2 id="capability-detail-empty-heading">Select a capability</h2>
|
||||
<p>Choose a result to inspect its input, output, and wrapper contract.</p>
|
||||
|
||||
@@ -165,7 +165,11 @@ describe("useCapabilityDiscovery", () => {
|
||||
client.list
|
||||
.mockResolvedValueOnce(page([summary("local.documents.read")], "page-2"))
|
||||
.mockResolvedValueOnce(
|
||||
page([summary("local.documents.read"), summary("local.documents.write")]),
|
||||
page([
|
||||
summary("local.documents.read"),
|
||||
summary("local.documents.write"),
|
||||
summary("local.documents.write"),
|
||||
]),
|
||||
);
|
||||
const { result } = renderHook(() => useCapabilityDiscovery());
|
||||
await waitFor(() => expect(result.current.nextCursor).toBe("page-2"));
|
||||
@@ -180,6 +184,36 @@ describe("useCapabilityDiscovery", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it("uses the applied filters when loading more after draft edits", async () => {
|
||||
client.list
|
||||
.mockResolvedValueOnce(page([summary("local.documents.read")], "page-2"))
|
||||
.mockResolvedValueOnce(page([summary("local.documents.write")]));
|
||||
const { result } = renderHook(() => useCapabilityDiscovery());
|
||||
await waitFor(() => expect(result.current.nextCursor).toBe("page-2"));
|
||||
|
||||
act(() => result.current.setQuery("draft-query"));
|
||||
act(() => result.current.setSourceId("draft-source"));
|
||||
act(() => result.current.loadMore());
|
||||
|
||||
await waitFor(() => expect(result.current.items).toHaveLength(2));
|
||||
expect(client.list).toHaveBeenLastCalledWith({ cursor: "page-2", limit: 50 });
|
||||
});
|
||||
|
||||
it("does not start a second load-more request while the page is pending", async () => {
|
||||
const nextPage = deferred<CapabilityPage>();
|
||||
client.list.mockResolvedValueOnce(page([summary("local.documents.read")], "page-2"));
|
||||
client.list.mockReturnValueOnce(nextPage.promise);
|
||||
const { result } = renderHook(() => useCapabilityDiscovery());
|
||||
await waitFor(() => expect(result.current.nextCursor).toBe("page-2"));
|
||||
|
||||
act(() => result.current.loadMore());
|
||||
act(() => result.current.loadMore());
|
||||
|
||||
expect(client.list).toHaveBeenCalledTimes(2);
|
||||
nextPage.resolve(page([summary("local.documents.write")]));
|
||||
await waitFor(() => expect(result.current.items).toHaveLength(2));
|
||||
});
|
||||
|
||||
it("loads the selected capability detail", async () => {
|
||||
client.list.mockResolvedValue(page([summary("local.documents.read")]));
|
||||
client.inspect.mockResolvedValue(detail("local.documents.read"));
|
||||
|
||||
@@ -28,10 +28,22 @@ export type CapabilityDiscoveryController = {
|
||||
|
||||
type DiscoveryState = Omit<CapabilityDiscoveryController, "setQuery" | "setSourceId" | "search" | "loadMore" | "inspect">;
|
||||
|
||||
const initialState: DiscoveryState = {
|
||||
type CapabilityFilters = {
|
||||
readonly query: string;
|
||||
readonly sourceId: string;
|
||||
};
|
||||
|
||||
type DiscoveryStateWithAppliedFilters = DiscoveryState & {
|
||||
readonly appliedQuery: string;
|
||||
readonly appliedSourceId: string;
|
||||
};
|
||||
|
||||
const initialState: DiscoveryStateWithAppliedFilters = {
|
||||
phase: "disconnected",
|
||||
query: "",
|
||||
sourceId: "",
|
||||
appliedQuery: "",
|
||||
appliedSourceId: "",
|
||||
items: [],
|
||||
selected: null,
|
||||
nextCursor: null,
|
||||
@@ -57,7 +69,13 @@ const appendUnique = (
|
||||
additions: ReadonlyArray<CapabilitySummary>,
|
||||
): ReadonlyArray<CapabilitySummary> => {
|
||||
const names = new Set(existing.map((item) => item.name));
|
||||
return [...existing, ...additions.filter((item) => !names.has(item.name))];
|
||||
const result = [...existing];
|
||||
for (const item of additions) {
|
||||
if (names.has(item.name)) continue;
|
||||
names.add(item.name);
|
||||
result.push(item);
|
||||
}
|
||||
return result;
|
||||
};
|
||||
|
||||
export const useCapabilityDiscovery = (): CapabilityDiscoveryController => {
|
||||
@@ -66,17 +84,19 @@ export const useCapabilityDiscovery = (): CapabilityDiscoveryController => {
|
||||
() => (readExecutor ? createCapabilityClient(readExecutor) : null),
|
||||
[readExecutor],
|
||||
);
|
||||
const [state, setState] = useState<DiscoveryState>(initialState);
|
||||
const [state, setState] = useState<DiscoveryStateWithAppliedFilters>(initialState);
|
||||
const listGenerationRef = useRef(0);
|
||||
const inspectGenerationRef = useRef(0);
|
||||
|
||||
const runList = useCallback(
|
||||
(query: string, sourceId: string, cursor: string | undefined, append: boolean): void => {
|
||||
(filters: CapabilityFilters, cursor: string | undefined, append: boolean): void => {
|
||||
if (!client) return;
|
||||
const generation = ++listGenerationRef.current;
|
||||
if (append === false) inspectGenerationRef.current++;
|
||||
setState((current) => ({
|
||||
...current,
|
||||
appliedQuery: filters.query,
|
||||
appliedSourceId: filters.sourceId,
|
||||
phase: "loading",
|
||||
items: append ? current.items : [],
|
||||
selected: append ? current.selected : null,
|
||||
@@ -85,7 +105,7 @@ export const useCapabilityDiscovery = (): CapabilityDiscoveryController => {
|
||||
}));
|
||||
|
||||
void client
|
||||
.list(requestParams(query, sourceId, cursor))
|
||||
.list(requestParams(filters.query, filters.sourceId, cursor))
|
||||
.then((page) => {
|
||||
if (generation !== listGenerationRef.current) return;
|
||||
setState((current) => ({
|
||||
@@ -123,7 +143,7 @@ export const useCapabilityDiscovery = (): CapabilityDiscoveryController => {
|
||||
return;
|
||||
}
|
||||
|
||||
runList(state.query, state.sourceId, undefined, false);
|
||||
runList({ query: state.query, sourceId: state.sourceId }, undefined, false);
|
||||
// The executor identity changes with the connected target. Query and source
|
||||
// filters are intentionally retained so reconnecting preserves the view.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
@@ -141,13 +161,23 @@ export const useCapabilityDiscovery = (): CapabilityDiscoveryController => {
|
||||
);
|
||||
|
||||
const search = useCallback(() => {
|
||||
runList(state.query, state.sourceId, undefined, false);
|
||||
runList({ query: state.query, sourceId: state.sourceId }, undefined, false);
|
||||
}, [runList, state.query, state.sourceId]);
|
||||
|
||||
const loadMore = useCallback(() => {
|
||||
if (!state.nextCursor) return;
|
||||
runList(state.query, state.sourceId, state.nextCursor, true);
|
||||
}, [runList, state.nextCursor, state.query, state.sourceId]);
|
||||
if (!state.nextCursor || state.phase === "loading") return;
|
||||
runList(
|
||||
{ query: state.appliedQuery, sourceId: state.appliedSourceId },
|
||||
state.nextCursor,
|
||||
true,
|
||||
);
|
||||
}, [
|
||||
runList,
|
||||
state.appliedQuery,
|
||||
state.appliedSourceId,
|
||||
state.nextCursor,
|
||||
state.phase,
|
||||
]);
|
||||
|
||||
const inspect = useCallback(
|
||||
(qualifiedName: string) => {
|
||||
|
||||
Reference in New Issue
Block a user