Skip to content

Commit 0a523be

Browse files
jc-systemslogiqjc-systemslogiq
committed
refactor: add support for loading API configurations by ID
- Introduced a new method `loadConfigById` in `ConfigManager` to load API configurations using their unique ID. - Refactored the dropdown and pin logic to work with ID - Added tests to verify the new functionality for loading configurations by ID.
1 parent 06055c3 commit 0a523be

File tree

7 files changed

+151
-45
lines changed

7 files changed

+151
-45
lines changed

‎src/core/config/ConfigManager.ts‎

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,17 +89,20 @@ export class ConfigManager {
8989
}
9090

9191
/**
92-
* Save a config with the given name
92+
* Save a config with the given name.
93+
* Preserves the ID from the input 'config' object if it exists,
94+
* otherwise generates a new one (for creation scenarios).
9395
*/
9496
async saveConfig(name: string, config: ApiConfiguration): Promise<void> {
9597
try {
9698
return await this.lock(async () => {
9799
const currentConfig = await this.readConfig()
98-
const existingConfig = currentConfig.apiConfigs[name]
100+
99101
currentConfig.apiConfigs[name] = {
100102
...config,
101-
id: existingConfig?.id || this.generateId(),
103+
id: config.id || this.generateId(),
102104
}
105+
103106
await this.writeConfig(currentConfig)
104107
})
105108
} catch (error) {
@@ -130,6 +133,34 @@ export class ConfigManager {
130133
}
131134
}
132135

136+
/**
137+
* Load a config by ID
138+
*/
139+
async loadConfigById(id: string): Promise<{ config: ApiConfiguration; name: string }> {
140+
try {
141+
return await this.lock(async () => {
142+
const config = await this.readConfig()
143+
144+
// Find the config with the matching ID
145+
const entry = Object.entries(config.apiConfigs).find(([_, apiConfig]) => apiConfig.id === id)
146+
147+
if (!entry) {
148+
throw new Error(`Config with ID '${id}' not found`)
149+
}
150+
151+
const [name, apiConfig] = entry
152+
153+
// Update current config name
154+
config.currentApiConfigName = name
155+
await this.writeConfig(config)
156+
157+
return { config: apiConfig, name }
158+
})
159+
} catch (error) {
160+
throw new Error(`Failed to load config by ID: ${error}`)
161+
}
162+
}
163+
133164
/**
134165
* Delete a config by name
135166
*/

‎src/core/webview/ClineProvider.ts‎

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1867,16 +1867,24 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
18671867
break
18681868
}
18691869

1870-
await this.configManager.saveConfig(newName, message.apiConfiguration)
1870+
// Load the old configuration to get its ID
1871+
const oldConfig = await this.configManager.loadConfig(oldName)
1872+
1873+
// Create a new configuration with the same ID
1874+
const newConfig = {
1875+
...message.apiConfiguration,
1876+
id: oldConfig.id, // Preserve the ID
1877+
}
1878+
1879+
// Save with the new name but same ID
1880+
await this.configManager.saveConfig(newName, newConfig)
18711881
await this.configManager.deleteConfig(oldName)
18721882

18731883
const listApiConfig = await this.configManager.listConfig()
1874-
const config = listApiConfig?.find((c) => c.name === newName)
18751884

18761885
// Update listApiConfigMeta first to ensure UI has latest data
18771886
await this.updateGlobalState("listApiConfigMeta", listApiConfig)
1878-
1879-
await Promise.all([this.updateGlobalState("currentApiConfigName", newName)])
1887+
await this.updateGlobalState("currentApiConfigName", newName)
18801888

18811889
await this.postStateToWebview()
18821890
} catch (error) {
@@ -1908,6 +1916,29 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
19081916
}
19091917
}
19101918
break
1919+
case "loadApiConfigurationById":
1920+
if (message.text) {
1921+
try {
1922+
const { config: apiConfig, name } = await this.configManager.loadConfigById(
1923+
message.text,
1924+
)
1925+
const listApiConfig = await this.configManager.listConfig()
1926+
1927+
await Promise.all([
1928+
this.updateGlobalState("listApiConfigMeta", listApiConfig),
1929+
this.updateGlobalState("currentApiConfigName", name),
1930+
this.updateApiConfiguration(apiConfig),
1931+
])
1932+
1933+
await this.postStateToWebview()
1934+
} catch (error) {
1935+
this.outputChannel.appendLine(
1936+
`Error load api configuration by ID: ${JSON.stringify(error, Object.getOwnPropertyNames(error), 2)}`,
1937+
)
1938+
vscode.window.showErrorMessage(t("common:errors.load_api_config"))
1939+
}
1940+
}
1941+
break
19111942
case "deleteApiConfiguration":
19121943
if (message.text) {
19131944
const answer = await vscode.window.showInformationMessage(

‎src/core/webview/__tests__/ClineProvider.test.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -700,6 +700,9 @@ describe("ClineProvider", () => {
700700

701701
provider.configManager = {
702702
loadConfig: jest.fn().mockResolvedValue({ apiProvider: "anthropic", id: "new-id" }),
703+
loadConfigById: jest
704+
.fn()
705+
.mockResolvedValue({ config: { apiProvider: "anthropic", id: "new-id" }, name: "new-config" }),
703706
listConfig: jest.fn().mockResolvedValue([{ name: "new-config", id: "new-id", apiProvider: "anthropic" }]),
704707
setModeConfig: jest.fn(),
705708
getModeConfigId: jest.fn().mockResolvedValue(undefined),
@@ -715,6 +718,35 @@ describe("ClineProvider", () => {
715718
expect(provider.configManager.setModeConfig).toHaveBeenCalledWith("architect", "new-id")
716719
})
717720

721+
test("load API configuration by ID works and updates mode config", async () => {
722+
await provider.resolveWebviewView(mockWebviewView)
723+
const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as jest.Mock).mock.calls[0][0]
724+
725+
provider.configManager = {
726+
loadConfigById: jest.fn().mockResolvedValue({
727+
config: { apiProvider: "anthropic", id: "config-id-123" },
728+
name: "config-by-id",
729+
}),
730+
listConfig: jest
731+
.fn()
732+
.mockResolvedValue([{ name: "config-by-id", id: "config-id-123", apiProvider: "anthropic" }]),
733+
setModeConfig: jest.fn(),
734+
getModeConfigId: jest.fn().mockResolvedValue(undefined),
735+
} as any
736+
737+
// First set the mode
738+
await messageHandler({ type: "mode", text: "architect" })
739+
740+
// Then load the config by ID
741+
await messageHandler({ type: "loadApiConfigurationById", text: "config-id-123" })
742+
743+
// Should save new config as default for architect mode
744+
expect(provider.configManager.setModeConfig).toHaveBeenCalledWith("architect", "config-id-123")
745+
746+
// Ensure the loadConfigById method was called with the correct ID
747+
expect(provider.configManager.loadConfigById).toHaveBeenCalledWith("config-id-123")
748+
})
749+
718750
test("handles browserToolEnabled setting", async () => {
719751
await provider.resolveWebviewView(mockWebviewView)
720752
const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as jest.Mock).mock.calls[0][0]

‎src/shared/WebviewMessage.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ export interface WebviewMessage {
1717
| "upsertApiConfiguration"
1818
| "deleteApiConfiguration"
1919
| "loadApiConfiguration"
20+
| "loadApiConfigurationById"
2021
| "renameApiConfiguration"
2122
| "getListApiConfiguration"
2223
| "customInstructions"

‎webview-ui/src/components/chat/ChatTextArea.tsx‎

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,16 @@ const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
7474
pinnedApiConfigs,
7575
togglePinnedApiConfig,
7676
} = useExtensionState()
77+
78+
// Find the ID and display text for the currently selected API configuration
79+
const { currentConfigId, displayName } = useMemo(() => {
80+
const currentConfig = listApiConfigMeta?.find((config) => config.name === currentApiConfigName)
81+
return {
82+
currentConfigId: currentConfig?.id || "",
83+
displayName: currentApiConfigName || "", // Use the name directly for display
84+
}
85+
}, [listApiConfigMeta, currentApiConfigName])
86+
7787
const [gitCommits, setGitCommits] = useState<any[]>([])
7888
const [showDropdown, setShowDropdown] = useState(false)
7989
const [fileSearchResults, setFileSearchResults] = useState<SearchResult[]>([])
@@ -964,23 +974,26 @@ const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
964974
{/* API configuration selector - flexible width */}
965975
<div className={cn("flex-1", "min-w-0", "overflow-hidden")}>
966976
<SelectDropdown
967-
value={currentApiConfigName || ""}
977+
value={currentConfigId}
968978
disabled={textAreaDisabled}
969979
title={t("chat:selectApiConfig")}
980+
placeholder={displayName} // Always show the current name
970981
options={[
971982
// Pinned items first
972983
...(listApiConfigMeta || [])
973-
.filter((config) => pinnedApiConfigs && pinnedApiConfigs[config.name])
984+
.filter((config) => pinnedApiConfigs && pinnedApiConfigs[config.id])
974985
.map((config) => ({
975-
value: config.name,
986+
value: config.id,
976987
label: config.name,
988+
name: config.name, // Keep name for comparison with currentApiConfigName
977989
type: DropdownOptionType.ITEM,
978990
pinned: true,
979-
})),
991+
}))
992+
.sort((a, b) => a.label.localeCompare(b.label)),
980993
// If we have pinned items and unpinned items, add a separator
981994
...(pinnedApiConfigs &&
982995
Object.keys(pinnedApiConfigs).length > 0 &&
983-
(listApiConfigMeta || []).some((config) => !pinnedApiConfigs[config.name])
996+
(listApiConfigMeta || []).some((config) => !pinnedApiConfigs[config.id])
984997
? [
985998
{
986999
value: "sep-pinned",
@@ -991,10 +1004,11 @@ const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
9911004
: []),
9921005
// Unpinned items sorted alphabetically
9931006
...(listApiConfigMeta || [])
994-
.filter((config) => !pinnedApiConfigs || !pinnedApiConfigs[config.name])
1007+
.filter((config) => !pinnedApiConfigs || !pinnedApiConfigs[config.id])
9951008
.map((config) => ({
996-
value: config.name,
1009+
value: config.id,
9971010
label: config.name,
1011+
name: config.name, // Keep name for comparison with currentApiConfigName
9981012
type: DropdownOptionType.ITEM,
9991013
pinned: false,
10001014
}))
@@ -1010,14 +1024,32 @@ const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
10101024
type: DropdownOptionType.ACTION,
10111025
},
10121026
]}
1013-
onChange={(value) => vscode.postMessage({ type: "loadApiConfiguration", text: value })}
1027+
onChange={(value) => {
1028+
if (value === "settingsButtonClicked") {
1029+
// Handle special actions
1030+
vscode.postMessage({
1031+
type: "loadApiConfiguration",
1032+
text: value,
1033+
})
1034+
} else {
1035+
// Use the ID directly with our new message type
1036+
vscode.postMessage({
1037+
type: "loadApiConfigurationById",
1038+
text: value,
1039+
})
1040+
}
1041+
}}
10141042
contentClassName="max-h-[300px] overflow-y-auto"
10151043
triggerClassName="w-full text-ellipsis overflow-hidden"
10161044
renderItem={(option) => {
10171045
if (option.type !== DropdownOptionType.ITEM) {
10181046
return option.label
10191047
}
10201048

1049+
// Get the config from listApiConfigMeta by ID
1050+
const config = listApiConfigMeta?.find((c) => c.id === option.value)
1051+
const isCurrentConfig = config?.name === currentApiConfigName
1052+
10211053
return (
10221054
<div className="flex items-center justify-between w-full">
10231055
<span>{option.label}</span>
@@ -1033,15 +1065,13 @@ const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
10331065
togglePinnedApiConfig(option.value)
10341066
vscode.postMessage({
10351067
type: "toggleApiConfigPin",
1036-
text: option.value,
1068+
text: option.value, // Send ID as text
10371069
})
10381070
}}
10391071
title={option.pinned ? t("chat:unpin") : t("chat:pin")}>
10401072
<Pin className="size-3" />
10411073
</div>
1042-
{option.value === currentApiConfigName && (
1043-
<Check className="size-4 p-0.5 ml-1" />
1044-
)}
1074+
{isCurrentConfig && <Check className="size-4 p-0.5 ml-1" />}
10451075
</div>
10461076
)
10471077
}}

‎webview-ui/src/components/ui/select-dropdown.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,10 @@ export const SelectDropdown = React.forwardRef<React.ElementRef<typeof DropdownM
6464
const [open, setOpen] = React.useState(false)
6565
const portalContainer = useRooPortal("roo-portal")
6666

67+
// If the selected option isn't in the list yet, but we have a placeholder, prioritize showing the placeholder
6768
const selectedOption = options.find((option) => option.value === value)
68-
const displayText = selectedOption?.label || placeholder || ""
69+
const displayText =
70+
value && !selectedOption && placeholder ? placeholder : selectedOption?.label || placeholder || ""
6971

7072
const handleSelect = (option: DropdownOption) => {
7173
if (option.type === DropdownOptionType.ACTION) {

‎webview-ui/src/context/ExtensionStateContext.tsx‎

Lines changed: 4 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -225,27 +225,6 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode
225225
setListApiConfigMeta(message.listApiConfig ?? [])
226226
break
227227
}
228-
case "toggleApiConfigPin": {
229-
if (message.text) {
230-
setState((prevState) => {
231-
const currentPinned = prevState.pinnedApiConfigs || {}
232-
const newPinned = { ...currentPinned }
233-
234-
// Toggle the pinned state
235-
if (currentPinned[message.text!]) {
236-
delete newPinned[message.text!]
237-
} else {
238-
newPinned[message.text!] = true
239-
}
240-
241-
return {
242-
...prevState,
243-
pinnedApiConfigs: newPinned,
244-
}
245-
})
246-
}
247-
break
248-
}
249228
}
250229
},
251230
[setListApiConfigMeta],
@@ -333,17 +312,17 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode
333312
setRemoteBrowserEnabled: (value) => setState((prevState) => ({ ...prevState, remoteBrowserEnabled: value })),
334313
setMaxReadFileLine: (value) => setState((prevState) => ({ ...prevState, maxReadFileLine: value })),
335314
setPinnedApiConfigs: (value) => setState((prevState) => ({ ...prevState, pinnedApiConfigs: value })),
336-
togglePinnedApiConfig: (configName) =>
315+
togglePinnedApiConfig: (configId) =>
337316
setState((prevState) => {
338317
const currentPinned = prevState.pinnedApiConfigs || {}
339318
const newPinned = {
340319
...currentPinned,
341-
[configName]: !currentPinned[configName],
320+
[configId]: !currentPinned[configId],
342321
}
343322

344323
// If the config is now unpinned, remove it from the object
345-
if (!newPinned[configName]) {
346-
delete newPinned[configName]
324+
if (!newPinned[configId]) {
325+
delete newPinned[configId]
347326
}
348327

349328
return { ...prevState, pinnedApiConfigs: newPinned }

0 commit comments

Comments
 (0)