Merge pull request #812 from grafana/fix-800-provider-switching-ui-consistency

sd2k · web-flow · commit 202bfb3003ca · 2025-10-22T13:58:21.000+01:00
diff --git a/packages/grafana-llm-app/src/components/AppConfig/LLMConfig.tsx b/packages/grafana-llm-app/src/components/AppConfig/LLMConfig.tsx
@@ -327,6 +327,7 @@ export function LLMConfig({
                       secretsSet={secretsSet}
                       onChangeSecrets={onChangeSecrets}
                       allowCustomPath={false}
+                      parentProvider={settings.provider}
                     />
                     <ModelConfig
                       provider={settings.provider ?? 'openai'}
@@ -382,6 +383,7 @@ export function LLMConfig({
                       secretsSet={secretsSet}
                       onChangeSecrets={onChangeSecrets}
                       allowCustomPath={true}
+                      parentProvider={settings.provider}
                     />
                     <ModelConfig
                       provider={settings.provider ?? 'custom'}
diff --git a/packages/grafana-llm-app/src/components/AppConfig/OpenAI.tsx b/packages/grafana-llm-app/src/components/AppConfig/OpenAI.tsx
@@ -34,15 +34,21 @@ export function OpenAIConfig({
   onChange,
   onChangeSecrets,
   allowCustomPath = false,
+  parentProvider,
 }: {
   settings: OpenAISettings;
   onChange: (settings: OpenAISettings) => void;
   secrets: Secrets;
   secretsSet: SecretsSet;
   onChangeSecrets: (secrets: Secrets) => void;
   allowCustomPath: boolean;
+  parentProvider?: ProviderType;
 }) {
   const s = useStyles2(getStyles);
+
+  // Get the effective provider - use the stored provider, or default to 'openai' if undefined
+  const effectiveProvider = settings.provider ?? 'openai';
+
   // Helper to update settings using the name of the HTML event.
   const onChangeField = (event: ChangeEvent<HTMLInputElement>) => {
     onChange({
@@ -80,7 +86,7 @@ export function OpenAIConfig({
 
   return (
     <FieldSet>
-      {settings.provider !== 'custom' && (
+      {parentProvider !== 'custom' && (
         <Field label="Provider">
           <Select
             data-testid={testIds.appConfig.provider}
@@ -90,31 +96,31 @@ export function OpenAIConfig({
                 { label: 'Azure OpenAI', value: 'azure' },
               ] as Array<SelectableValue<ProviderType>>
             }
-            value={settings.provider ?? 'openai'}
+            value={effectiveProvider}
             onChange={(e) => onChangeProvider(e.value as ProviderType)}
             width={60}
           />
         </Field>
       )}
 
       <Field
-        label={settings.provider === 'azure' ? 'Azure OpenAI Language API Endpoint' : 'API URL'}
+        label={effectiveProvider === 'azure' ? 'Azure OpenAI Language API Endpoint' : 'API URL'}
         className={s.marginTop}
       >
         <Input
           width={60}
           name="url"
           data-testid={testIds.appConfig.openAIUrl}
-          value={settings.provider === 'openai' ? OPENAI_API_URL : settings.url}
+          value={effectiveProvider === 'openai' ? OPENAI_API_URL : settings.url}
           placeholder={
-            settings.provider === 'azure'
+            effectiveProvider === 'azure'
               ? AZURE_OPENAI_URL_TEMPLATE
-              : settings.provider === 'openai'
+              : effectiveProvider === 'openai'
                 ? OPENAI_API_URL
                 : `https://llm.domain.com`
           }
           onChange={onChangeField}
-          disabled={settings.provider === 'openai'}
+          disabled={effectiveProvider === 'openai' && parentProvider !== 'custom'}
         />
       </Field>
 
@@ -145,20 +151,20 @@ export function OpenAIConfig({
         </Field>
       )}
 
-      <Field label={settings.provider === 'azure' ? 'Azure OpenAI Key' : 'API Key'}>
+      <Field label={effectiveProvider === 'azure' ? 'Azure OpenAI Key' : 'API Key'}>
         <SecretInput
           width={60}
           data-testid={testIds.appConfig.openAIKey}
           name="openAIKey"
           value={secrets.openAIKey}
           isConfigured={secretsSet.openAIKey ?? false}
-          placeholder={settings.provider === 'azure' ? '' : 'sk-...'}
+          placeholder={effectiveProvider === 'azure' ? '' : 'sk-...'}
           onChange={(e) => onChangeSecrets({ ...secrets, openAIKey: e.currentTarget.value })}
           onReset={() => onChangeSecrets({ ...secrets, openAIKey: '' })}
         />
       </Field>
 
-      {settings.provider === 'openai' && (
+      {effectiveProvider === 'openai' && (
         <Field label="API Organization ID">
           <Input
             width={60}
@@ -171,7 +177,7 @@ export function OpenAIConfig({
         </Field>
       )}
 
-      {settings.provider === 'azure' && (
+      {effectiveProvider === 'azure' && (
         <Field label="Azure OpenAI Model Mapping" description="Mapping from model name to Azure deployment name.">
           <AzureModelDeploymentConfig
             modelMapping={settings.azureModelMapping ?? []}
diff --git a/packages/grafana-llm-app/tests/provider-switching.spec.ts b/packages/grafana-llm-app/tests/provider-switching.spec.ts
@@ -0,0 +1,193 @@
+/**
+ * End-to-end tests for provider switching UI consistency.
+ *
+ * This test suite verifies that when switching between LLM providers (OpenAI, Azure, Custom, Anthropic),
+ * the UI maintains consistent state and properly shows/hides fields based on the selected provider.
+ *
+ * Key behaviors tested:
+ * - Provider dropdown visibility (hidden for custom, visible for openai/azure)
+ * - URL field enabled/disabled state (disabled for openai, enabled for azure/custom)
+ * - Organization ID field visibility (only for openai, not for azure/custom)
+ * - Custom API path option (only available for custom provider)
+ * - Field consistency when switching between providers
+ *
+ * This addresses a bug where fields would have inconsistent state depending on whether
+ * settings.openAI.provider was set, particularly when switching to 'custom' provider.
+ */
+import { test, expect } from '@grafana/plugin-e2e';
+import { testIds } from '../src/components/testIds';
+
+test.describe('Provider Switching UI Consistency', () => {
+  test.beforeEach(async ({ page }) => {
+    // Navigate to the plugin configuration page
+    await page.goto('/plugins/grafana-llm-app');
+
+    // Wait for the configuration form to load
+    await page.waitForSelector(`[data-testid="${testIds.appConfig.container}"]`, {
+      state: 'visible',
+      timeout: 10000,
+    });
+  });
+
+  test('should show consistent UI when first switching to custom provider', async ({ page }) => {
+    // Click on the Custom API card
+    const customCard = page.locator('text=Use a Custom API').locator('..');
+    await customCard.click();
+
+    // Wait for the card to be selected
+    await page.waitForTimeout(500);
+
+    // Verify provider dropdown is NOT visible (it should be hidden for custom)
+    const providerDropdown = page.getByTestId(testIds.appConfig.provider);
+    await expect(providerDropdown).not.toBeVisible();
+
+    // Verify URL field is visible and enabled
+    const urlField = page.getByTestId(testIds.appConfig.openAIUrl);
+    await expect(urlField).toBeVisible();
+    await expect(urlField).toBeEnabled();
+
+    // Verify Organization ID field is visible (should default to openai mode)
+    const orgIdField = page.getByTestId(testIds.appConfig.openAIOrganizationID);
+    await expect(orgIdField).toBeVisible();
+
+    // Verify API Key field is visible
+    const apiKeyField = page.getByTestId(testIds.appConfig.openAIKey);
+    await expect(apiKeyField).toBeVisible();
+
+    // Verify custom API path checkbox is visible
+    const customPathCheckbox = page.getByTestId(testIds.appConfig.customizeOpenAIApiPath);
+    await expect(customPathCheckbox).toBeVisible();
+  });
+
+  test('should maintain consistent UI when switching from openai to custom and back', async ({ page }) => {
+    // First, select OpenAI
+    const openaiCard = page.locator('text=Use OpenAI-compatible API').locator('..');
+    await openaiCard.click();
+    await page.waitForTimeout(500);
+
+    // Verify provider dropdown IS visible for OpenAI
+    let providerDropdown = page.getByTestId(testIds.appConfig.provider);
+    await expect(providerDropdown).toBeVisible();
+
+    // Verify URL field is disabled for OpenAI (always uses api.openai.com)
+    let urlField = page.getByTestId(testIds.appConfig.openAIUrl);
+    await expect(urlField).toBeVisible();
+    await expect(urlField).toBeDisabled();
+
+    // Now switch to Custom
+    const customCard = page.locator('text=Use a Custom API').locator('..');
+    await customCard.click();
+    await page.waitForTimeout(500);
+
+    // Verify provider dropdown is now hidden
+    providerDropdown = page.getByTestId(testIds.appConfig.provider);
+    await expect(providerDropdown).not.toBeVisible();
+
+    // Verify URL field is now enabled
+    urlField = page.getByTestId(testIds.appConfig.openAIUrl);
+    await expect(urlField).toBeEnabled();
+
+    // Verify Organization ID field is still visible
+    const orgIdField = page.getByTestId(testIds.appConfig.openAIOrganizationID);
+    await expect(orgIdField).toBeVisible();
+
+    // Switch back to OpenAI
+    await openaiCard.click();
+    await page.waitForTimeout(500);
+
+    // Verify provider dropdown is visible again
+    providerDropdown = page.getByTestId(testIds.appConfig.provider);
+    await expect(providerDropdown).toBeVisible();
+
+    // Verify URL field is disabled again
+    urlField = page.getByTestId(testIds.appConfig.openAIUrl);
+    await expect(urlField).toBeDisabled();
+  });
+
+  test('should hide provider dropdown when in custom mode', async ({ page }) => {
+    // Select OpenAI first
+    const openaiCard = page.locator('text=Use OpenAI-compatible API').locator('..');
+    await openaiCard.click();
+    await page.waitForTimeout(500);
+
+    // Provider dropdown should be visible
+    const providerDropdown = page.getByTestId(testIds.appConfig.provider);
+    await expect(providerDropdown).toBeVisible();
+
+    // Select Custom
+    const customCard = page.locator('text=Use a Custom API').locator('..');
+    await customCard.click();
+    await page.waitForTimeout(500);
+
+    // Provider dropdown should be hidden
+    await expect(providerDropdown).not.toBeVisible();
+  });
+
+  test('should switch to Anthropic provider correctly', async ({ page }) => {
+    // Select Anthropic card
+    const anthropicCard = page.locator('text=Use Anthropic API').locator('..');
+    await anthropicCard.click();
+    await page.waitForTimeout(500);
+
+    // Verify Anthropic-specific fields are visible
+    const anthropicUrl = page.getByTestId(testIds.appConfig.anthropicUrl);
+    const anthropicKey = page.getByTestId(testIds.appConfig.anthropicKey);
+
+    await expect(anthropicUrl).toBeVisible();
+    await expect(anthropicKey).toBeVisible();
+
+    // OpenAI-specific fields should not be visible
+    const openaiUrl = page.getByTestId(testIds.appConfig.openAIUrl);
+    await expect(openaiUrl).not.toBeVisible();
+  });
+
+  test('should disable LLM features correctly', async ({ page }) => {
+    // Select the disable card
+    const disableCard = page.locator('text=Disable all LLM features in Grafana').locator('..');
+    await disableCard.click();
+    await page.waitForTimeout(500);
+
+    // Configuration fields should not be visible
+    const openaiUrl = page.getByTestId(testIds.appConfig.openAIUrl);
+    const anthropicUrl = page.getByTestId(testIds.appConfig.anthropicUrl);
+
+    await expect(openaiUrl).not.toBeVisible();
+    await expect(anthropicUrl).not.toBeVisible();
+
+    // Re-enable by selecting OpenAI
+    const openaiCard = page.locator('text=Use OpenAI-compatible API').locator('..');
+    await openaiCard.click();
+    await page.waitForTimeout(500);
+
+    // OpenAI fields should be visible again
+    await expect(openaiUrl).toBeVisible();
+  });
+
+  test('should show confirmation dialog when switching providers with existing model configs', async ({ page }) => {
+    // This test assumes there might be existing model configurations
+    // First, select OpenAI and configure it
+    const openaiCard = page.locator('text=Use OpenAI-compatible API').locator('..');
+    await openaiCard.click();
+    await page.waitForTimeout(500);
+
+    // Try switching to Anthropic
+    const anthropicCard = page.locator('text=Use Anthropic API').locator('..');
+    await anthropicCard.click();
+
+    // Check if confirmation dialog appears (it may not if there are no model configs)
+    const confirmDialog = page.locator('text=Switch LLM Provider?');
+
+    // If dialog appears, handle it
+    const dialogVisible = await confirmDialog.isVisible().catch(() => false);
+    if (dialogVisible) {
+      // Click "Switch Provider" to confirm
+      const confirmButton = page.locator('button', { hasText: 'Switch Provider' });
+      await confirmButton.click();
+      await page.waitForTimeout(500);
+
+      // Verify we switched to Anthropic
+      const anthropicUrl = page.getByTestId(testIds.appConfig.anthropicUrl);
+      await expect(anthropicUrl).toBeVisible();
+    }
+  });
+});
