All display of items seems to work properly

This commit is contained in:
Smartsheet-JB-Brown 2025-04-15 07:26:27 -07:00
parent ab0e7cd81e
commit f551fe2476
7 changed files with 439 additions and 177 deletions

View file

@ -228,10 +228,15 @@ export async function handlePackageManagerMessages(
}
case "filterPackageManagerItems": {
console.log("DEBUG: Handling filterPackageManagerItems message", {
filters: message.filters,
hasItems: packageManagerManager.getCurrentItems().length > 0,
})
if (message.filters) {
try {
// Get current items from the manager
const items = packageManagerManager.getCurrentItems()
console.log("DEBUG: Current items before filtering:", items.length)
// Apply filters using the manager's filtering logic
const filteredItems = packageManagerManager.filterItems(items, {
@ -239,13 +244,22 @@ export async function handlePackageManagerMessages(
search: message.filters.search,
tags: message.filters.tags,
})
// Get current state and merge with filtered items
console.log("DEBUG: Filtered items:", {
beforeCount: items.length,
afterCount: filteredItems.length,
filters: message.filters,
})
// Get current state and merge filtered items
const currentState = await provider.getStateToPostToWebview()
await provider.postMessageToWebview({
type: "state",
state: { ...currentState, packageManagerItems: filteredItems },
state: {
...currentState,
packageManagerItems: filteredItems,
},
})
console.log("DEBUG: State update sent with filtered items:", filteredItems.length)
console.log("DEBUG: State update sent with filtered items:", filteredItems.length)
} catch (error) {
console.error("Package Manager: Error filtering items:", error)
vscode.window.showErrorMessage("Failed to filter package manager items")

View file

@ -316,6 +316,15 @@ export class PackageManagerManager {
items: PackageManagerItem[],
filters: { type?: ComponentType; search?: string; tags?: string[] },
): PackageManagerItem[] {
console.log("DEBUG: Starting filterItems", {
itemCount: items.length,
filters: {
type: filters.type,
search: filters.search,
tags: filters.tags,
},
})
// Helper function to normalize text for case/whitespace-insensitive comparison
const normalizeText = (text: string) => text.toLowerCase().replace(/\s+/g, " ").trim()
@ -328,125 +337,119 @@ export class PackageManagerManager {
return normalizeText(text).includes(normalizeText(searchTerm))
}
const filteredItems = items.map((originalItem) => {
// Create a deep clone of the item to avoid modifying the original
return JSON.parse(JSON.stringify(originalItem)) as PackageManagerItem
})
// Create a deep clone of all items
const clonedItems = items.map((originalItem) => JSON.parse(JSON.stringify(originalItem)) as PackageManagerItem)
console.log("Initial items:", JSON.stringify(filteredItems))
return filteredItems.filter((item) => {
// For packages, handle differently based on filters
if (item.type === "package") {
// If we have a type filter
if (filters.type) {
// Check if the package itself matches the type filter
const packageTypeMatch = item.type === filters.type
console.log("Initial items:", JSON.stringify(clonedItems))
// Check subcomponents if they exist
let hasMatchingSubcomponents = false
if (item.items && item.items.length > 0) {
// Mark subcomponents with matchInfo based on type
item.items.forEach((subItem) => {
const subTypeMatch = subItem.type === filters.type
subItem.matchInfo = {
matched: subTypeMatch,
matchReason: {
typeMatch: subTypeMatch,
},
}
})
// Apply filters
const filteredItems = clonedItems.filter((item) => {
// Check if item itself matches type filter
const itemTypeMatch = !filters.type || item.type === filters.type
// Check if any subcomponents match
hasMatchingSubcomponents = item.items.some((subItem) => subItem.matchInfo?.matched)
}
// Check if any subcomponents match type filter
const subcomponentTypeMatch =
item.items?.some((subItem) => !filters.type || subItem.type === filters.type) ?? false
// Set package matchInfo
item.matchInfo = {
matched: packageTypeMatch || hasMatchingSubcomponents,
matchReason: {
typeMatch: packageTypeMatch,
hasMatchingSubcomponents,
},
}
// Keep package if it or any of its subcomponents match the type filter
return packageTypeMatch || hasMatchingSubcomponents
}
// For search term
if (searchTerm) {
// Check package and subcomponents
const nameMatch = containsSearchTerm(item.name)
const descMatch = containsSearchTerm(item.description)
// Process subcomponents if they exist
if (item.items && item.items.length > 0) {
// Add matchInfo to each subcomponent
item.items.forEach((subItem) => {
if (!subItem.metadata) {
subItem.matchInfo = { matched: false }
return
}
const subNameMatch = containsSearchTerm(subItem.metadata.name)
const subDescMatch = containsSearchTerm(subItem.metadata.description)
console.log(`Checking subcomponent: ${subItem.metadata.name}`)
console.log(`Search term: ${searchTerm}`)
console.log(`Name match: ${subNameMatch}, Desc match: ${subDescMatch}`)
if (subNameMatch || subDescMatch) {
subItem.matchInfo = {
matched: true,
matchReason: {
nameMatch: subNameMatch,
descriptionMatch: subDescMatch,
},
}
} else {
subItem.matchInfo = { matched: false }
}
})
}
// Check if any subcomponents matched
const hasMatchingSubcomponents = item.items?.some((subItem) => subItem.matchInfo?.matched) ?? false
// Set package matchInfo
item.matchInfo = {
matched: nameMatch || descMatch || hasMatchingSubcomponents,
matchReason: {
nameMatch,
descriptionMatch: descMatch,
hasMatchingSubcomponents,
},
}
// Only keep package if it or its subcomponents match the exact search term
const packageMatches = nameMatch || descMatch
const subcomponentMatches = hasMatchingSubcomponents
return packageMatches || subcomponentMatches
}
// No search term, everything matches
item.matchInfo = { matched: true }
if (item.items) {
item.items.forEach((subItem) => {
subItem.matchInfo = { matched: true }
})
}
return true
}
// For non-packages
if (filters.type && item.type !== filters.type) {
// Type filter - include if item or any subcomponent matches
if (filters.type && !itemTypeMatch && !subcomponentTypeMatch) {
return false
}
// Search filter
if (searchTerm) {
return containsSearchTerm(item.name) || containsSearchTerm(item.description)
const nameMatch = containsSearchTerm(item.name)
const descMatch = containsSearchTerm(item.description)
const subcomponentMatch =
item.items?.some(
(subItem) =>
subItem.metadata &&
(containsSearchTerm(subItem.metadata.name) ||
containsSearchTerm(subItem.metadata.description)),
) ?? false
return nameMatch || descMatch || subcomponentMatch
}
return true
})
console.log("Filtered items:", {
before: clonedItems.length,
after: filteredItems.length,
filters,
})
// Add match info to filtered items
return filteredItems.map((item) => {
const nameMatch = searchTerm ? containsSearchTerm(item.name) : true
const descMatch = searchTerm ? containsSearchTerm(item.description) : true
const typeMatch = filters.type ? item.type === filters.type : true
// Process subcomponents first to determine if any match
let hasMatchingSubcomponents = false
if (item.items) {
item.items = item.items.map((subItem) => {
// Calculate matches
const subNameMatch =
searchTerm && subItem.metadata ? containsSearchTerm(subItem.metadata.name) : true
const subDescMatch =
searchTerm && subItem.metadata ? containsSearchTerm(subItem.metadata.description) : true
// Only calculate type match if type filter is active
const subTypeMatch = filters.type ? subItem.type === filters.type : false
// Determine if item matches based on active filters
const subMatched = filters.type
? subNameMatch || subDescMatch || subTypeMatch
: subNameMatch || subDescMatch
if (subMatched) {
hasMatchingSubcomponents = true
// Only include matchReason if the item matches
const matchReason: Record<string, boolean> = {
nameMatch: subNameMatch,
descriptionMatch: subDescMatch,
}
// Only include type match in reason if type filter is active
if (filters.type) {
matchReason.typeMatch = subTypeMatch
}
subItem.matchInfo = {
matched: true,
matchReason,
}
} else {
subItem.matchInfo = {
matched: false,
}
}
return subItem
})
}
const matchReason: Record<string, boolean> = {
nameMatch,
descriptionMatch: descMatch,
}
// Only include typeMatch and hasMatchingSubcomponents in matchReason if relevant
if (filters.type) {
matchReason.typeMatch = typeMatch
}
if (hasMatchingSubcomponents) {
matchReason.hasMatchingSubcomponents = true
}
item.matchInfo = {
matched: nameMatch || descMatch || typeMatch || hasMatchingSubcomponents,
matchReason,
}
return item
})
}
/**

View file

@ -6,6 +6,110 @@ import * as path from "path"
import * as vscode from "vscode"
describe("PackageManagerManager", () => {
describe("filterItems", () => {
// Create a mock context with required properties
const mockContext = {
globalStorageUri: {
fsPath: path.resolve(__dirname, "../../../../mock/settings/path"),
},
extensionPath: path.resolve(__dirname, "../../../../"),
subscriptions: [],
workspaceState: {
get: jest.fn(),
update: jest.fn(),
},
globalState: {
get: jest.fn(),
update: jest.fn(),
},
asAbsolutePath: jest.fn((p) => p),
storagePath: "",
logPath: "",
extensionUri: { fsPath: "" },
environmentVariableCollection: {},
extensionMode: 1,
storageUri: { fsPath: "" },
} as unknown as vscode.ExtensionContext
let manager: PackageManagerManager
beforeEach(() => {
// Create a new manager instance with the mock context for each test
manager = new PackageManagerManager(mockContext)
})
it("should correctly filter items by search term", () => {
const items: PackageManagerItem[] = [
{
name: "Test Item 1",
description: "First test item",
type: "mode",
url: "test1",
repoUrl: "test1",
},
{
name: "Another Item",
description: "Second item",
type: "mode",
url: "test2",
repoUrl: "test2",
},
]
const filtered = manager.filterItems(items, { search: "test" })
expect(filtered).toHaveLength(1)
expect(filtered[0].name).toBe("Test Item 1")
expect(filtered[0].matchInfo?.matched).toBe(true)
})
it("should correctly filter items by type", () => {
const items: PackageManagerItem[] = [
{
name: "Mode Item",
description: "A mode",
type: "mode",
url: "test1",
repoUrl: "test1",
},
{
name: "Server Item",
description: "A server",
type: "mcp server",
url: "test2",
repoUrl: "test2",
},
]
const filtered = manager.filterItems(items, { type: "mode" })
expect(filtered).toHaveLength(1)
expect(filtered[0].name).toBe("Mode Item")
expect(filtered[0].matchInfo?.matchReason?.typeMatch).toBe(true)
})
it("should preserve original items when filtering", () => {
const items: PackageManagerItem[] = [
{
name: "Test Item 1",
description: "First test item",
type: "mode",
url: "test1",
repoUrl: "test1",
},
{
name: "Another Item",
description: "Second item",
type: "mode",
url: "test2",
repoUrl: "test2",
},
]
const originalItemsJson = JSON.stringify(items)
manager.filterItems(items, { search: "test" })
expect(JSON.stringify(items)).toBe(originalItemsJson)
})
})
let manager: PackageManagerManager
let metadataScanner: MetadataScanner
let realItems: PackageManagerItem[]

View file

@ -10,7 +10,7 @@ export const DEFAULT_PACKAGE_MANAGER_REPO_URL = "https://github.com/RooVetGit/Ro
/**
* Default package manager repository name
*/
export const DEFAULT_PACKAGE_MANAGER_REPO_NAME = "Roo Code Package Manager Template"
export const DEFAULT_PACKAGE_MANAGER_REPO_NAME = "Roo Code"
/**
* Default package manager source

View file

@ -22,10 +22,13 @@ const PackageManagerView: React.FC<PackageManagerViewProps> = ({ onDone }) => {
useEffect(() => {
console.log("State updated:", {
allItems: state.allItems,
displayItems: state.displayItems,
itemsLength: state.allItems.length,
showingEmptyState: state.allItems.length === 0,
displayItemsLength: state.displayItems?.length,
showingEmptyState: (state.displayItems || state.allItems).length === 0,
filters: state.filters,
})
}, [state.allItems])
}, [state.allItems, state.displayItems, state.filters])
// Fetch items on mount
useEffect(() => {
@ -238,7 +241,9 @@ const PackageManagerView: React.FC<PackageManagerViewProps> = ({ onDone }) => {
{(() => {
// Debug log state
const items = state.allItems || []
const items = checkFilterActive(state.filters)
? state.displayItems || []
: state.allItems || []
const isEmpty = items.length === 0
const isLoading = state.isFetching
console.log("=== Rendering PackageManagerView ===")
@ -251,8 +256,8 @@ const PackageManagerView: React.FC<PackageManagerViewProps> = ({ onDone }) => {
filters: state.filters,
})
// Show loading state if fetching
if (isLoading) {
// Show loading state if fetching and not filtering
if (isLoading && !checkFilterActive(state.filters)) {
console.log("Rendering loading state due to isFetching=true")
return (
<div className="flex flex-col items-center justify-center h-64 text-vscode-descriptionForeground">

View file

@ -5,6 +5,7 @@ import { DEFAULT_PACKAGE_MANAGER_SOURCE } from "../../../../src/services/package
export interface ViewState {
allItems: PackageManagerItem[]
displayItems?: PackageManagerItem[] // Items currently being displayed (filtered or all)
isFetching: boolean
activeTab: "browse" | "sources"
refreshingUrls: string[]
@ -51,6 +52,7 @@ export class PackageManagerViewStateManager {
constructor() {
this.state = {
allItems: [],
displayItems: [] as PackageManagerItem[],
isFetching: false,
activeTab: "browse",
refreshingUrls: [],
@ -96,9 +98,12 @@ export class PackageManagerViewStateManager {
console.log("=== State Change Notification ===")
console.log("Current state:", {
allItems: this.state.allItems,
displayItems: this.state.displayItems,
itemsLength: this.state.allItems.length,
displayItemsLength: this.state.displayItems?.length,
isFetching: this.state.isFetching,
activeTab: this.state.activeTab,
filters: this.state.filters,
})
// Create a deep copy to ensure React sees changes
@ -106,9 +111,12 @@ export class PackageManagerViewStateManager {
console.log("Notifying handlers with state:", {
allItems: newState.allItems,
displayItems: newState.displayItems,
itemsLength: newState.allItems.length,
displayItemsLength: newState.displayItems?.length,
isFetching: newState.isFetching,
activeTab: newState.activeTab,
filters: newState.filters,
})
this.stateChangeHandlers.forEach((handler) => {
@ -136,23 +144,24 @@ export class PackageManagerViewStateManager {
})
// Create a new state object to ensure React sees the change
this.state = {
const newState = {
...this.state,
isFetching: true,
}
console.log("After setting isFetching:", {
isFetching: this.state.isFetching,
allItems: this.state.allItems.length,
})
// Clear any existing timeout before starting new fetch
this.clearFetchTimeout()
// Update state and notify before starting fetch
this.state = newState
this.notifyStateChange()
// Set timeout for fetch operation
this.fetchTimeoutId = setTimeout(() => {
void this.transition({ type: "FETCH_ERROR" })
}, this.FETCH_TIMEOUT)
// Request items from extension
vscode.postMessage({
type: "fetchPackageManagerItems",
bool: true,
@ -171,15 +180,26 @@ export class PackageManagerViewStateManager {
receivedItems: items.length,
})
// Clear any existing timeout
this.clearFetchTimeout()
// Create a new state object to ensure React sees the change
this.state = {
// Create a new state object
const newState = {
...this.state,
isFetching: false,
allItems: [...items],
displayItems: this.isFilterActive() ? this.state.displayItems : [...items],
}
// If filters are active, apply them to the new items
if (this.isFilterActive()) {
const { filterItems } = await import("./selectors")
newState.displayItems = filterItems(items, this.state.filters)
}
// Update state and notify
this.state = newState
console.log("After state update:", {
isFetching: this.state.isFetching,
allItems: this.state.allItems.length,
@ -193,35 +213,51 @@ export class PackageManagerViewStateManager {
case "FETCH_ERROR": {
this.clearFetchTimeout()
this.state.isFetching = false
// Create a new state object to ensure React sees the change
this.state = {
...this.state,
isFetching: false,
}
this.notifyStateChange()
break
}
case "SET_ACTIVE_TAB": {
const { tab } = transition.payload as TransitionPayloads["SET_ACTIVE_TAB"]
this.state.activeTab = tab
// Create a new state object
const newState = {
...this.state,
activeTab: tab,
}
// Add default source when switching to sources tab if no sources exist
if (tab === "sources" && this.state.sources.length === 0) {
this.state.sources = [DEFAULT_PACKAGE_MANAGER_SOURCE]
if (tab === "sources" && newState.sources.length === 0) {
newState.sources = [DEFAULT_PACKAGE_MANAGER_SOURCE]
vscode.postMessage({
type: "packageManagerSources",
sources: [DEFAULT_PACKAGE_MANAGER_SOURCE],
} as WebviewMessage)
}
// Update state and notify
this.state = newState
this.notifyStateChange()
// Handle browse tab switch
if (tab === "browse") {
// Clear any existing timeouts
this.clearFetchTimeout()
// Always fetch when switching to browse if sources were modified
if (this.sourcesModified) {
this.sourcesModified = false // Reset the flag
void this.transition({ type: "FETCH_ITEMS" })
} else {
} else if (this.state.allItems.length === 0) {
// Only fetch if we don't have any items yet
if (this.state.allItems.length === 0) {
void this.transition({ type: "FETCH_ITEMS" })
}
void this.transition({ type: "FETCH_ITEMS" })
}
}
break
@ -234,40 +270,63 @@ export class PackageManagerViewStateManager {
newFilters: filters,
})
this.state.filters = {
...this.state.filters,
...filters,
// Create new state with updated filters
const newState = {
...this.state,
filters: {
...this.state.filters,
...filters,
},
}
// Check if all filters are being cleared
const isFilterClearing = filters
? Object.values(filters).every(
(value) => value === "" || (Array.isArray(value) && value.length === 0),
)
: true
// Update display items based on filter state
if (isFilterClearing) {
console.log("Clearing all filters, restoring original items")
newState.displayItems = [...newState.allItems]
} else {
// Apply client-side filtering immediately
const { filterItems } = await import("./selectors")
newState.displayItems = filterItems(newState.allItems, newState.filters)
}
// Update state and notify
this.state = newState
this.notifyStateChange()
const isActive = this.isFilterActive()
console.log("Filter state:", {
filters: this.state.filters,
isActive,
hasTimeout: !!this.filterTimeoutId,
displayItemsCount: this.state.displayItems?.length ?? 0,
isFilterClearing,
})
if (isActive) {
// Always use debounce
if (this.filterTimeoutId) {
console.log("Clearing existing filter timeout")
clearTimeout(this.filterTimeoutId)
}
console.log("Setting up new filter timeout")
this.filterTimeoutId = setTimeout(() => {
console.log("Filter timeout executed, sending message")
vscode.postMessage({
type: "filterPackageManagerItems",
filters: {
type: this.state.filters.type || undefined,
search: this.state.filters.search || undefined,
tags: this.state.filters.tags.length > 0 ? this.state.filters.tags : undefined,
},
} as WebviewMessage)
this.filterTimeoutId = undefined
}, this.FILTER_DEBOUNCE)
// Debounce server-side filter request
if (this.filterTimeoutId) {
console.log("Clearing existing filter timeout")
clearTimeout(this.filterTimeoutId)
}
console.log("Setting up new filter timeout")
this.filterTimeoutId = setTimeout(() => {
console.log("Filter timeout executed, sending message")
vscode.postMessage({
type: "filterPackageManagerItems",
filters: {
type: this.state.filters.type || undefined,
search: this.state.filters.search || undefined,
tags: this.state.filters.tags.length > 0 ? this.state.filters.tags : undefined,
},
} as WebviewMessage)
this.filterTimeoutId = undefined
}, this.FILTER_DEBOUNCE)
console.log("=== UPDATE_FILTERS Finished ===")
break
}
@ -337,17 +396,23 @@ export class PackageManagerViewStateManager {
}
private clearFetchTimeout(): void {
// Clear fetch timeout
if (this.fetchTimeoutId) {
clearTimeout(this.fetchTimeoutId)
this.fetchTimeoutId = undefined
}
// Also clear any pending filter timeout to avoid race conditions
if (this.filterTimeoutId) {
clearTimeout(this.filterTimeoutId)
this.filterTimeoutId = undefined
}
}
private isFilterActive(): boolean {
return !!(this.state.filters.type || this.state.filters.search || this.state.filters.tags.length > 0)
}
public handleMessage(message: any): void {
public async handleMessage(message: any): Promise<void> {
console.log("=== Handling Message ===", {
messageType: message.type,
hasPackageManagerItems: !!message.state?.packageManagerItems,
@ -379,17 +444,35 @@ export class PackageManagerViewStateManager {
this.notifyStateChange()
}
if (message.state?.isFetching) {
console.log("State indicates fetching, transitioning to FETCH_ITEMS")
void this.transition({
type: "FETCH_ITEMS",
})
} else if (message.state?.packageManagerItems) {
if (message.state?.packageManagerItems) {
console.log("State includes items, transitioning to FETCH_COMPLETE")
void this.transition({
type: "FETCH_COMPLETE",
payload: { items: message.state.packageManagerItems },
})
// Always update allItems with the latest items
const newState = {
...this.state,
allItems: message.state.packageManagerItems,
displayItems: this.isFilterActive() ? this.state.displayItems : message.state.packageManagerItems,
isFetching: false,
}
// If filters are active, apply them to the new items
if (this.isFilterActive()) {
const { filterItems } = await import("./selectors")
newState.displayItems = filterItems(newState.allItems, this.state.filters)
// Send filter message
vscode.postMessage({
type: "filterPackageManagerItems",
filters: {
type: this.state.filters.type || undefined,
search: this.state.filters.search || undefined,
tags: this.state.filters.tags.length > 0 ? this.state.filters.tags : undefined,
},
} as WebviewMessage)
}
// Update state and notify
this.state = newState
this.notifyStateChange()
}
}

View file

@ -53,6 +53,7 @@ describe("PackageManagerViewStateManager", () => {
const state = manager.getState()
expect(state).toEqual({
allItems: [],
displayItems: [],
isFetching: false,
activeTab: "browse",
refreshingUrls: [],
@ -421,7 +422,22 @@ describe("PackageManagerViewStateManager", () => {
})
})
it("should not send filter message if no filters are active", async () => {
it("should send filter message even when filters are cleared", async () => {
// First set some filters
await manager.transition({
type: "UPDATE_FILTERS",
payload: {
filters: {
type: "mode",
search: "test",
},
},
})
// Clear mock to ignore the first filter message
;(vscode.postMessage as jest.Mock).mockClear()
// Clear filters
await manager.transition({
type: "UPDATE_FILTERS",
payload: {
@ -436,12 +452,15 @@ describe("PackageManagerViewStateManager", () => {
// Fast-forward past debounce time
jest.advanceTimersByTime(300)
// Should not send filter message
expect(vscode.postMessage).not.toHaveBeenCalledWith(
expect.objectContaining({
type: "filterPackageManagerItems",
}),
)
// Should send filter message with empty filters
expect(vscode.postMessage).toHaveBeenCalledWith({
type: "filterPackageManagerItems",
filters: {
type: undefined,
search: undefined,
tags: undefined,
},
})
})
})
@ -760,6 +779,40 @@ describe("PackageManagerViewStateManager", () => {
})
describe("Filter Transitions", () => {
it("should preserve original items when receiving filtered results", async () => {
// Set up initial items
const initialItems = [
createTestItem({ name: "Item 1" }),
createTestItem({ name: "Item 2" }),
createTestItem({ name: "Item 3" }),
]
await manager.transition({
type: "FETCH_COMPLETE",
payload: { items: initialItems },
})
// Apply a filter
await manager.transition({
type: "UPDATE_FILTERS",
payload: { filters: { search: "Item 1" } },
})
// Fast-forward past debounce time
jest.advanceTimersByTime(300)
// Simulate receiving filtered results
manager.handleMessage({
type: "state",
state: {
packageManagerItems: [initialItems[0]], // Only Item 1
},
})
// Verify original items are preserved
const state = manager.getState()
expect(state.allItems).toEqual(initialItems)
})
it("should handle UPDATE_FILTERS transition", async () => {
const filters = {
type: "mode",