fix(FN-1987): move skills search to top and improve catalog fallback
- Move the shared skills search input to the top of SkillsView so it applies to discovered skills and catalog results - Add client-side discovered skills filtering by name/path with a dedicated filtered-empty-state message - Extend skills adapter fallback logic to use unauthenticated catalog search when authenticated requests return 400, 401, or 403 - Add dashboard component and adapter tests covering discovered filtering behavior and authenticated-endpoint fallback scenarios
This commit is contained in:
@@ -30,6 +30,15 @@ export function SkillsView({ projectId, addToast, onClose }: SkillsViewProps) {
|
|||||||
const debounceRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
const debounceRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||||
const [debouncedQuery, setDebouncedQuery] = useState("");
|
const [debouncedQuery, setDebouncedQuery] = useState("");
|
||||||
|
|
||||||
|
// Client-side filtering for discovered skills
|
||||||
|
const filteredDiscoveredSkills = searchQuery.trim()
|
||||||
|
? discoveredSkills.filter(
|
||||||
|
(s) =>
|
||||||
|
s.name.toLowerCase().includes(searchQuery.toLowerCase()) ||
|
||||||
|
s.relativePath.toLowerCase().includes(searchQuery.toLowerCase())
|
||||||
|
)
|
||||||
|
: discoveredSkills;
|
||||||
|
|
||||||
// Fetch discovered skills
|
// Fetch discovered skills
|
||||||
const loadDiscoveredSkills = useCallback(async () => {
|
const loadDiscoveredSkills = useCallback(async () => {
|
||||||
setIsLoadingDiscovered(true);
|
setIsLoadingDiscovered(true);
|
||||||
@@ -152,6 +161,18 @@ export function SkillsView({ projectId, addToast, onClose }: SkillsViewProps) {
|
|||||||
|
|
||||||
{/* Scrollable content area */}
|
{/* Scrollable content area */}
|
||||||
<div className="skills-view-content">
|
<div className="skills-view-content">
|
||||||
|
{/* Search — at top for both sections */}
|
||||||
|
<div className="skills-view-search">
|
||||||
|
<input
|
||||||
|
type="text"
|
||||||
|
className="form-input"
|
||||||
|
placeholder="Search skills..."
|
||||||
|
value={searchQuery}
|
||||||
|
onChange={(e) => handleSearchChange(e.target.value)}
|
||||||
|
aria-label="Search skills"
|
||||||
|
/>
|
||||||
|
</div>
|
||||||
|
|
||||||
{/* Discovered Skills Section */}
|
{/* Discovered Skills Section */}
|
||||||
<section className="skills-view-section" aria-labelledby="discovered-skills-title">
|
<section className="skills-view-section" aria-labelledby="discovered-skills-title">
|
||||||
<h3 id="discovered-skills-title" className="skills-view-section-title">
|
<h3 id="discovered-skills-title" className="skills-view-section-title">
|
||||||
@@ -167,9 +188,13 @@ export function SkillsView({ projectId, addToast, onClose }: SkillsViewProps) {
|
|||||||
<div className="skills-view-empty">
|
<div className="skills-view-empty">
|
||||||
<p>No skills discovered in this project.</p>
|
<p>No skills discovered in this project.</p>
|
||||||
</div>
|
</div>
|
||||||
|
) : filteredDiscoveredSkills.length === 0 ? (
|
||||||
|
<div className="skills-view-empty">
|
||||||
|
<p>No discovered skills match your search.</p>
|
||||||
|
</div>
|
||||||
) : (
|
) : (
|
||||||
<div className="skills-view-list">
|
<div className="skills-view-list">
|
||||||
{discoveredSkills.map((skill) => (
|
{filteredDiscoveredSkills.map((skill) => (
|
||||||
<div key={skill.id} className="skills-view-item">
|
<div key={skill.id} className="skills-view-item">
|
||||||
<div className="skills-view-item-info">
|
<div className="skills-view-item-info">
|
||||||
<span className="skills-view-item-name">{skill.name}</span>
|
<span className="skills-view-item-name">{skill.name}</span>
|
||||||
@@ -198,18 +223,6 @@ export function SkillsView({ projectId, addToast, onClose }: SkillsViewProps) {
|
|||||||
Skills Catalog
|
Skills Catalog
|
||||||
</h3>
|
</h3>
|
||||||
|
|
||||||
{/* Search */}
|
|
||||||
<div className="skills-view-search">
|
|
||||||
<input
|
|
||||||
type="text"
|
|
||||||
className="form-input"
|
|
||||||
placeholder="Search skills..."
|
|
||||||
value={searchQuery}
|
|
||||||
onChange={(e) => handleSearchChange(e.target.value)}
|
|
||||||
aria-label="Search skills catalog"
|
|
||||||
/>
|
|
||||||
</div>
|
|
||||||
|
|
||||||
{/* Catalog Content */}
|
{/* Catalog Content */}
|
||||||
{catalogError ? (
|
{catalogError ? (
|
||||||
<div className="skills-view-error">
|
<div className="skills-view-error">
|
||||||
|
|||||||
@@ -620,4 +620,74 @@ describe("SkillsView", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("discovered skills filtering", () => {
|
||||||
|
it("filters discovered skills by search query", async () => {
|
||||||
|
render(<SkillsView addToast={mockAddToast} onClose={onClose} />);
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByText("test-skill")).toBeTruthy();
|
||||||
|
expect(screen.getByText("another-skill")).toBeTruthy();
|
||||||
|
});
|
||||||
|
|
||||||
|
const searchInput = screen.getByPlaceholderText("Search skills...");
|
||||||
|
fireEvent.change(searchInput, { target: { value: "test-skill" } });
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByText("test-skill")).toBeTruthy();
|
||||||
|
expect(screen.queryByText("another-skill")).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("shows filtered empty state when no discovered skills match search", async () => {
|
||||||
|
render(<SkillsView addToast={mockAddToast} onClose={onClose} />);
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByText("test-skill")).toBeTruthy();
|
||||||
|
});
|
||||||
|
|
||||||
|
const searchInput = screen.getByPlaceholderText("Search skills...");
|
||||||
|
fireEvent.change(searchInput, { target: { value: "zzz-nonexistent" } });
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByText("No discovered skills match your search.")).toBeTruthy();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("shows original empty state when no skills are discovered", async () => {
|
||||||
|
mockFetchDiscoveredSkills.mockResolvedValue([]);
|
||||||
|
|
||||||
|
render(<SkillsView addToast={mockAddToast} onClose={onClose} />);
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByText("No skills discovered in this project.")).toBeTruthy();
|
||||||
|
});
|
||||||
|
|
||||||
|
// Search should not override the "no skills discovered" empty state
|
||||||
|
const searchInput = screen.getByPlaceholderText("Search skills...");
|
||||||
|
fireEvent.change(searchInput, { target: { value: "test" } });
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByText("No skills discovered in this project.")).toBeTruthy();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("filters discovered skills by relativePath", async () => {
|
||||||
|
render(<SkillsView addToast={mockAddToast} onClose={onClose} />);
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByText("test-skill")).toBeTruthy();
|
||||||
|
expect(screen.getByText("another-skill")).toBeTruthy();
|
||||||
|
});
|
||||||
|
|
||||||
|
// Search by path instead of name
|
||||||
|
const searchInput = screen.getByPlaceholderText("Search skills...");
|
||||||
|
fireEvent.change(searchInput, { target: { value: "another-skill" } });
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.queryByText("test-skill")).toBeNull();
|
||||||
|
expect(screen.getByText("another-skill")).toBeTruthy();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -29388,9 +29388,9 @@ html .column.drag-over * {
|
|||||||
transform: translateX(18px);
|
transform: translateX(18px);
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Catalog search */
|
/* Skills search — at top */
|
||||||
.skills-view-search {
|
.skills-view-search {
|
||||||
margin-bottom: var(--space-md);
|
margin-bottom: var(--space-lg);
|
||||||
}
|
}
|
||||||
|
|
||||||
.skills-view-search .form-input {
|
.skills-view-search .form-input {
|
||||||
|
|||||||
214
packages/dashboard/src/__tests__/skills-adapter.test.ts
Normal file
214
packages/dashboard/src/__tests__/skills-adapter.test.ts
Normal file
@@ -0,0 +1,214 @@
|
|||||||
|
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||||
|
import { createSkillsAdapter } from "../skills-adapter.js";
|
||||||
|
|
||||||
|
describe("createSkillsAdapter - fetchCatalog fallback behavior", () => {
|
||||||
|
const originalFetch = globalThis.fetch;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.restoreAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
globalThis.fetch = originalFetch;
|
||||||
|
delete process.env.SKILLS_SH_TOKEN;
|
||||||
|
});
|
||||||
|
|
||||||
|
it("falls back to public search endpoint when authenticated endpoint returns 400", async () => {
|
||||||
|
process.env.SKILLS_SH_TOKEN = "test-token";
|
||||||
|
|
||||||
|
let fetchCallCount = 0;
|
||||||
|
globalThis.fetch = vi.fn().mockImplementation((url: string | URL | Request) => {
|
||||||
|
const urlStr = typeof url === "string" ? url : url.toString();
|
||||||
|
fetchCallCount++;
|
||||||
|
if (urlStr.includes("/api/v1/skills")) {
|
||||||
|
return Promise.resolve({
|
||||||
|
ok: false,
|
||||||
|
status: 400,
|
||||||
|
statusText: "Bad Request",
|
||||||
|
json: () => Promise.resolve(null),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
return Promise.resolve({
|
||||||
|
ok: true,
|
||||||
|
status: 200,
|
||||||
|
headers: new Map([["content-type", "application/json"]]),
|
||||||
|
json: () =>
|
||||||
|
Promise.resolve({
|
||||||
|
skills: [{ id: "s1", name: "Found Skill", skillId: "s1" }],
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
}) as unknown as typeof fetch;
|
||||||
|
|
||||||
|
const adapter = createSkillsAdapter({
|
||||||
|
packageManager: { resolve: vi.fn().mockResolvedValue({ skills: [] }) },
|
||||||
|
getSettingsPath: vi.fn().mockReturnValue("/tmp/settings.json"),
|
||||||
|
});
|
||||||
|
|
||||||
|
const result = await adapter.fetchCatalog({ limit: 20, query: "test" });
|
||||||
|
|
||||||
|
expect(fetchCallCount).toBe(2);
|
||||||
|
expect("entries" in result).toBe(true);
|
||||||
|
if ("entries" in result) {
|
||||||
|
expect(result.entries).toHaveLength(1);
|
||||||
|
expect(result.entries[0]!.name).toBe("Found Skill");
|
||||||
|
expect(result.auth.fallbackUsed).toBe(true);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("falls back to public search endpoint on 401", async () => {
|
||||||
|
process.env.SKILLS_SH_TOKEN = "test-token";
|
||||||
|
|
||||||
|
let fetchCallCount = 0;
|
||||||
|
globalThis.fetch = vi.fn().mockImplementation((url: string | URL | Request) => {
|
||||||
|
const urlStr = typeof url === "string" ? url : url.toString();
|
||||||
|
fetchCallCount++;
|
||||||
|
if (urlStr.includes("/api/v1/skills")) {
|
||||||
|
return Promise.resolve({
|
||||||
|
ok: false,
|
||||||
|
status: 401,
|
||||||
|
statusText: "Unauthorized",
|
||||||
|
json: () => Promise.resolve(null),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
return Promise.resolve({
|
||||||
|
ok: true,
|
||||||
|
status: 200,
|
||||||
|
headers: new Map([["content-type", "application/json"]]),
|
||||||
|
json: () =>
|
||||||
|
Promise.resolve({
|
||||||
|
skills: [{ id: "s2", name: "Fallback Skill", skillId: "s2" }],
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
}) as unknown as typeof fetch;
|
||||||
|
|
||||||
|
const adapter = createSkillsAdapter({
|
||||||
|
packageManager: { resolve: vi.fn().mockResolvedValue({ skills: [] }) },
|
||||||
|
getSettingsPath: vi.fn().mockReturnValue("/tmp/settings.json"),
|
||||||
|
});
|
||||||
|
|
||||||
|
const result = await adapter.fetchCatalog({ limit: 20, query: "test" });
|
||||||
|
|
||||||
|
expect(fetchCallCount).toBe(2);
|
||||||
|
expect("entries" in result).toBe(true);
|
||||||
|
if ("entries" in result) {
|
||||||
|
expect(result.entries).toHaveLength(1);
|
||||||
|
expect(result.entries[0]!.name).toBe("Fallback Skill");
|
||||||
|
expect(result.auth.fallbackUsed).toBe(true);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("falls back to public search endpoint on 403", async () => {
|
||||||
|
process.env.SKILLS_SH_TOKEN = "test-token";
|
||||||
|
|
||||||
|
let fetchCallCount = 0;
|
||||||
|
globalThis.fetch = vi.fn().mockImplementation((url: string | URL | Request) => {
|
||||||
|
const urlStr = typeof url === "string" ? url : url.toString();
|
||||||
|
fetchCallCount++;
|
||||||
|
if (urlStr.includes("/api/v1/skills")) {
|
||||||
|
return Promise.resolve({
|
||||||
|
ok: false,
|
||||||
|
status: 403,
|
||||||
|
statusText: "Forbidden",
|
||||||
|
json: () => Promise.resolve(null),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
return Promise.resolve({
|
||||||
|
ok: true,
|
||||||
|
status: 200,
|
||||||
|
headers: new Map([["content-type", "application/json"]]),
|
||||||
|
json: () =>
|
||||||
|
Promise.resolve({
|
||||||
|
skills: [{ id: "s3", name: "Forbidden Fallback", skillId: "s3" }],
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
}) as unknown as typeof fetch;
|
||||||
|
|
||||||
|
const adapter = createSkillsAdapter({
|
||||||
|
packageManager: { resolve: vi.fn().mockResolvedValue({ skills: [] }) },
|
||||||
|
getSettingsPath: vi.fn().mockReturnValue("/tmp/settings.json"),
|
||||||
|
});
|
||||||
|
|
||||||
|
const result = await adapter.fetchCatalog({ limit: 20, query: "test" });
|
||||||
|
|
||||||
|
expect(fetchCallCount).toBe(2);
|
||||||
|
expect("entries" in result).toBe(true);
|
||||||
|
if ("entries" in result) {
|
||||||
|
expect(result.entries).toHaveLength(1);
|
||||||
|
expect(result.entries[0]!.name).toBe("Forbidden Fallback");
|
||||||
|
expect(result.auth.fallbackUsed).toBe(true);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns UpstreamError when authenticated endpoint returns 500", async () => {
|
||||||
|
process.env.SKILLS_SH_TOKEN = "test-token";
|
||||||
|
|
||||||
|
let fetchCallCount = 0;
|
||||||
|
globalThis.fetch = vi.fn().mockImplementation((url: string | URL | Request) => {
|
||||||
|
const urlStr = typeof url === "string" ? url : url.toString();
|
||||||
|
fetchCallCount++;
|
||||||
|
if (urlStr.includes("/api/v1/skills")) {
|
||||||
|
return Promise.resolve({
|
||||||
|
ok: false,
|
||||||
|
status: 500,
|
||||||
|
statusText: "Internal Server Error",
|
||||||
|
json: () => Promise.resolve(null),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
// This should NOT be called
|
||||||
|
return Promise.resolve({
|
||||||
|
ok: true,
|
||||||
|
status: 200,
|
||||||
|
headers: new Map([["content-type", "application/json"]]),
|
||||||
|
json: () =>
|
||||||
|
Promise.resolve({
|
||||||
|
skills: [],
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
}) as unknown as typeof fetch;
|
||||||
|
|
||||||
|
const adapter = createSkillsAdapter({
|
||||||
|
packageManager: { resolve: vi.fn().mockResolvedValue({ skills: [] }) },
|
||||||
|
getSettingsPath: vi.fn().mockReturnValue("/tmp/settings.json"),
|
||||||
|
});
|
||||||
|
|
||||||
|
const result = await adapter.fetchCatalog({ limit: 20, query: "test" });
|
||||||
|
|
||||||
|
expect(fetchCallCount).toBe(1);
|
||||||
|
expect("error" in result).toBe(true);
|
||||||
|
if ("error" in result) {
|
||||||
|
expect(result.code).toBe("upstream_http_error");
|
||||||
|
expect(result.error).toContain("500");
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("uses public search endpoint when no token is present", async () => {
|
||||||
|
// Ensure no token
|
||||||
|
delete process.env.SKILLS_SH_TOKEN;
|
||||||
|
|
||||||
|
globalThis.fetch = vi.fn().mockResolvedValue({
|
||||||
|
ok: true,
|
||||||
|
status: 200,
|
||||||
|
headers: new Map([["content-type", "application/json"]]),
|
||||||
|
json: () =>
|
||||||
|
Promise.resolve({
|
||||||
|
skills: [{ id: "s4", name: "Public Skill", skillId: "s4" }],
|
||||||
|
}),
|
||||||
|
}) as unknown as typeof fetch;
|
||||||
|
|
||||||
|
const adapter = createSkillsAdapter({
|
||||||
|
packageManager: { resolve: vi.fn().mockResolvedValue({ skills: [] }) },
|
||||||
|
getSettingsPath: vi.fn().mockReturnValue("/tmp/settings.json"),
|
||||||
|
});
|
||||||
|
|
||||||
|
const result = await adapter.fetchCatalog({ limit: 20, query: "test" });
|
||||||
|
|
||||||
|
expect(globalThis.fetch).toHaveBeenCalledTimes(1);
|
||||||
|
expect("entries" in result).toBe(true);
|
||||||
|
if ("entries" in result) {
|
||||||
|
expect(result.entries).toHaveLength(1);
|
||||||
|
expect(result.entries[0]!.name).toBe("Public Skill");
|
||||||
|
expect(result.auth.tokenPresent).toBe(false);
|
||||||
|
expect(result.auth.fallbackUsed).toBe(false);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -429,8 +429,8 @@ export function createSkillsAdapter(options: {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
// 401/403 from authenticated request - fall back to public search endpoint
|
// 400/401/403 from authenticated request - fall back to public search endpoint
|
||||||
if (authResponse.status === 401 || authResponse.status === 403) {
|
if (authResponse.status === 400 || authResponse.status === 401 || authResponse.status === 403) {
|
||||||
return fetchPublicCatalog(searchUrl, {
|
return fetchPublicCatalog(searchUrl, {
|
||||||
mode: "fallback-unauthenticated",
|
mode: "fallback-unauthenticated",
|
||||||
tokenPresent: true,
|
tokenPresent: true,
|
||||||
|
|||||||
Reference in New Issue
Block a user