feat(edit-sources): Editing existing DW sources (PostHog#27934)

frontend/src/scenes/data-warehouse/external/forms/SourceForm.tsx

@@ -1,7 +1,7 @@
 import { LemonDivider, LemonFileInput, LemonInput, LemonSelect, LemonSwitch, LemonTextArea } from '@posthog/lemon-ui'
-import { Form, Group } from 'kea-forms'
+import { FieldName, Form, Group } from 'kea-forms'
 import { LemonField } from 'lib/lemon-ui/LemonField'
-import React from 'react'
+import React, { useEffect } from 'react'
 
 import { SourceConfig, SourceFieldConfig } from '~/types'
 
@@ -13,14 +13,25 @@ export interface SourceFormProps {
     sourceConfig: SourceConfig
     showPrefix?: boolean
     jobInputs?: Record<string, any>
+    setSourceConfigValue?: (key: FieldName, value: any) => void
 }
 
 const CONNECTION_STRING_DEFAULT_PORT = {
     Postgres: 5432,
 }
 
-const sourceFieldToElement = (field: SourceFieldConfig, sourceConfig: SourceConfig, lastValue?: any): JSX.Element => {
+const sourceFieldToElement = (
+    field: SourceFieldConfig,
+    sourceConfig: SourceConfig,
+    lastValue?: any,
+    isUpdateMode?: boolean
+): JSX.Element => {
+    // It doesn't make sense for this to show on an update to an existing connection since we likely just want to change
+    // a field or two. There is also some divergence in creates vs. updates that make this a bit more complex to handle.
     if (field.type === 'text' && field.name === 'connection_string') {
+        if (isUpdateMode) {
+            return <React.Fragment key={field.name} />
+        }
         return (
             <React.Fragment key={field.name}>
                 <LemonField name={field.name} label={field.label}>
@@ -38,7 +49,7 @@ const sourceFieldToElement = (field: SourceFieldConfig, sourceConfig: SourceConf
 
                                 if (isValid) {
                                     sourceWizardLogic.actions.setSourceConnectionDetailsValue(
-                                        ['payload', 'dbname'],
+                                        ['payload', 'database'],
                                         database || ''
                                     )
                                     sourceWizardLogic.actions.setSourceConnectionDetailsValue(
@@ -126,13 +137,16 @@ const sourceFieldToElement = (field: SourceFieldConfig, sourceConfig: SourceConf
     if (field.type === 'textarea') {
         return (
             <LemonField key={field.name} name={field.name} label={field.label}>
-                <LemonTextArea
-                    className="ph-ignore-input"
-                    data-attr={field.name}
-                    placeholder={field.placeholder}
-                    minRows={4}
-                    defaultValue={lastValue}
-                />
+                {({ value, onChange }) => (
+                    <LemonTextArea
+                        className="ph-ignore-input"
+                        data-attr={field.name}
+                        placeholder={field.placeholder}
+                        minRows={4}
+                        value={value || ''}
+                        onChange={onChange}
+                    />
+                )}
             </LemonField>
         )
     }
@@ -172,8 +186,7 @@ const sourceFieldToElement = (field: SourceFieldConfig, sourceConfig: SourceConf
                     data-attr={field.name}
                     placeholder={field.placeholder}
                     type={field.type as 'text'}
-                    defaultValue={lastValue}
-                    value={value ?? ''}
+                    value={value || ''}
                     onChange={onChange}
                 />
             )}
@@ -189,12 +202,27 @@ export default function SourceFormContainer(props: SourceFormProps): JSX.Element
     )
 }
 
-export function SourceFormComponent({ sourceConfig, showPrefix = true, jobInputs }: SourceFormProps): JSX.Element {
+export function SourceFormComponent({
+    sourceConfig,
+    showPrefix = true,
+    jobInputs,
+    setSourceConfigValue,
+}: SourceFormProps): JSX.Element {
+    useEffect(() => {
+        if (jobInputs && setSourceConfigValue) {
+            for (const input of Object.keys(jobInputs || {})) {
+                setSourceConfigValue(['payload', input], jobInputs[input])
+            }
+        }
+    }, [JSON.stringify(jobInputs), setSourceConfigValue])
+
+    const isUpdateMode = !!setSourceConfigValue
+
     return (
         <div className="space-y-4">
             <Group name="payload">
                 {SOURCE_DETAILS[sourceConfig.name].fields.map((field) =>
-                    sourceFieldToElement(field, sourceConfig, jobInputs?.[field.name])
+                    sourceFieldToElement(field, sourceConfig, jobInputs?.[field.name], isUpdateMode)
                 )}
             </Group>
             {showPrefix && (

frontend/src/scenes/data-warehouse/new/NewSourceWizard.tsx

@@ -3,7 +3,7 @@ import { useActions, useValues } from 'kea'
 import { PageHeader } from 'lib/components/PageHeader'
 import { FEATURE_FLAGS } from 'lib/constants'
 import { featureFlagLogic } from 'lib/logic/featureFlagLogic'
-import { useCallback } from 'react'
+import { useCallback, useEffect } from 'react'
 import { SceneExport } from 'scenes/sceneTypes'
 
 import { ManualLinkSourceType, SourceConfig } from '~/types'
@@ -52,20 +52,17 @@ interface NewSourcesWizardProps {
 export function NewSourcesWizard({ onComplete }: NewSourcesWizardProps): JSX.Element {
     const wizardLogic = sourceWizardLogic({ onComplete })
 
-    const {
-        modalTitle,
-        modalCaption,
-        isWrapped,
-        currentStep,
-        isLoading,
-        canGoBack,
-        canGoNext,
-        nextButtonText,
-        showSkipButton,
-    } = useValues(wizardLogic)
-    const { onBack, onSubmit } = useActions(wizardLogic)
+    const { modalTitle, modalCaption, isWrapped, currentStep, isLoading, canGoBack, canGoNext, nextButtonText } =
+        useValues(wizardLogic)
+    const { onBack, onSubmit, onClear } = useActions(wizardLogic)
     const { tableLoading: manualLinkIsLoading } = useValues(dataWarehouseTableLogic)
 
+    useEffect(() => {
+        return () => {
+            onClear()
+        }
+    }, [onClear])
+
     const footer = useCallback(() => {
         if (currentStep === 1) {
             return null
@@ -96,7 +93,7 @@ export function NewSourcesWizard({ onComplete }: NewSourcesWizardProps): JSX.Ele
                 </LemonButton>
             </div>
         )
-    }, [currentStep, isLoading, manualLinkIsLoading, canGoNext, canGoBack, nextButtonText, showSkipButton])
+    }, [currentStep, canGoBack, onBack, isLoading, manualLinkIsLoading, canGoNext, nextButtonText, onSubmit])
 
     return (
         <>

frontend/src/scenes/data-warehouse/new/sourceWizardLogic.tsx

@@ -51,14 +51,14 @@ export const SOURCE_DETAILS: Record<ExternalDataSourceType, SourceConfig> = {
         caption: <Caption />,
         fields: [
             {
-                name: 'account_id',
+                name: 'stripe_account_id',
                 label: 'Account id',
                 type: 'text',
                 required: false,
-                placeholder: 'acct_...',
+                placeholder: 'stripe_account_id',
             },
             {
-                name: 'client_secret',
+                name: 'stripe_secret_key',
                 label: 'Client secret',
                 type: 'password',
                 required: true,
@@ -103,7 +103,7 @@ export const SOURCE_DETAILS: Record<ExternalDataSourceType, SourceConfig> = {
                 placeholder: '5432',
             },
             {
-                name: 'dbname',
+                name: 'database',
                 label: 'Database',
                 type: 'text',
                 required: true,
@@ -234,7 +234,7 @@ export const SOURCE_DETAILS: Record<ExternalDataSourceType, SourceConfig> = {
                 placeholder: '3306',
             },
             {
-                name: 'dbname',
+                name: 'database',
                 label: 'Database',
                 type: 'text',
                 required: true,
@@ -383,7 +383,7 @@ export const SOURCE_DETAILS: Record<ExternalDataSourceType, SourceConfig> = {
                 placeholder: '1433',
             },
             {
-                name: 'dbname',
+                name: 'database',
                 label: 'Database',
                 type: 'text',
                 required: true,
@@ -872,21 +872,19 @@ export const sourceWizardLogic = kea<sourceWizardLogicType>([
             {
                 setDatabaseSchemas: (_, { schemas }) => schemas,
                 toggleSchemaShouldSync: (state, { schema, shouldSync }) => {
-                    const newSchema = state.map((s) => ({
+                    return state.map((s) => ({
                         ...s,
                         should_sync: s.table === schema.table ? shouldSync : s.should_sync,
                     }))
-                    return newSchema
                 },
                 updateSchemaSyncType: (state, { schema, syncType, incrementalField, incrementalFieldType }) => {
-                    const newSchema = state.map((s) => ({
+                    return state.map((s) => ({
                         ...s,
                         sync_type: s.table === schema.table ? syncType : s.sync_type,
                         incremental_field: s.table === schema.table ? incrementalField : s.incremental_field,
                         incremental_field_type:
                             s.table === schema.table ? incrementalFieldType : s.incremental_field_type,
                     }))
-                    return newSchema
                 },
             },
         ],
@@ -1304,9 +1302,7 @@ export const sourceWizardLogic = kea<sourceWizardLogicType>([
                                         fileReader.onerror = (e) => reject(e)
                                         fileReader.readAsText(payload['payload'][name][0])
                                     })
-                                    const jsonConfig = JSON.parse(loadedFile)
-
-                                    fieldPayload[name] = jsonConfig
+                                    fieldPayload[name] = JSON.parse(loadedFile)
                                 } catch (e) {
                                     return lemonToast.error('File is not valid')
                                 }
@@ -1375,7 +1371,7 @@ export const getErrorsForFields = (
                 }
             } else {
                 errorsObj[field.name] = {}
-                const selection = valueObj[field.name]['selection']
+                const selection = valueObj[field.name]?.['selection']
                 field.options
                     .find((n) => n.value === selection)
                     ?.fields?.forEach((f) => validateField(f, valueObj[field.name], errorsObj[field.name]))

frontend/src/scenes/data-warehouse/settings/source/SourceConfiguration.tsx

@@ -1,8 +1,10 @@
 import { LemonBanner, LemonButton, LemonSkeleton } from '@posthog/lemon-ui'
-import { BindLogic, useValues } from 'kea'
+import { BindLogic, useActions, useValues } from 'kea'
 import { Form } from 'kea-forms'
 import { SourceFormComponent, SourceFormProps } from 'scenes/data-warehouse/external/forms/SourceForm'
 
+import { ExternalDataSourceType } from '~/types'
+
 import { dataWarehouseSourceSettingsLogic } from './dataWarehouseSourceSettingsLogic'
 
 interface SourceConfigurationProps {
@@ -26,24 +28,33 @@ interface UpdateSourceConnectionFormContainerProps extends SourceFormProps {
     id: string
 }
 
+const supportedEditingSourceTypes: ExternalDataSourceType[] = ['MSSQL', 'MySQL', 'Postgres', 'Stripe']
+
 function UpdateSourceConnectionFormContainer(props: UpdateSourceConnectionFormContainerProps): JSX.Element {
     const { source, sourceLoading } = useValues(dataWarehouseSourceSettingsLogic({ id: props.id }))
+    const { setSourceConfigValue } = useActions(dataWarehouseSourceSettingsLogic)
 
-    if (source?.source_type !== 'MSSQL' && source?.source_type !== 'MySQL' && source?.source_type !== 'Postgres') {
+    if (!source?.source_type || !supportedEditingSourceTypes.includes(source?.source_type)) {
         return (
             <LemonBanner type="warning" className="mt-2">
                 <p>
-                    Only Postgres, MSSQL, and MySQL are configurable. Please delete and recreate your source if you need
-                    to connect to a new source of the same type.
+                    Only {supportedEditingSourceTypes.slice(0, -1).join(', ')}, and {supportedEditingSourceTypes.at(-1)}{' '}
+                    are configurable. Please delete and recreate your source if you need to connect to a new source of
+                    the same type.
                 </p>
             </LemonBanner>
         )
     }
+
     return (
         <>
             <span className="block mb-2">Overwrite your existing configuration here</span>
             <Form logic={dataWarehouseSourceSettingsLogic} formKey="sourceConfig" enableFormOnSubmit>
-                <SourceFormComponent {...props} />
+                <SourceFormComponent
+                    {...props}
+                    jobInputs={source?.job_inputs}
+                    setSourceConfigValue={setSourceConfigValue}
+                />
                 <div className="mt-4 flex flex-row justify-end gap-2">
                     <LemonButton
                         loading={sourceLoading && !source}

posthog/warehouse/api/external_data_source.py

@@ -185,9 +185,35 @@ class Meta:
             "schemas",
             "prefix",
         ]
-        extra_kwargs = {
-            "job_inputs": {"write_only": True},
+
+    """
+    This method is used to remove sensitive fields from the response.
+    IMPORTANT: This method should be updated when a new source type is added to allow for editing of the new source.
+    """
+
+    def to_representation(self, instance):
+        representation = super().to_representation(instance)
+
+        # non-sensitive fields
+        whitelisted_keys = {
+            # stripe
+            "stripe_account_id",
+            # sql
+            "database",
+            "host",
+            "port",
+            "user",
+            "schema",
+            "ssh-tunnel",
+            "use_ssl",
         }
+        job_inputs = representation.get("job_inputs", {})
+        if isinstance(job_inputs, dict):
+            for key in list(job_inputs.keys()):  # Use list() to avoid modifying dict during iteration
+                if key not in whitelisted_keys:
+                    job_inputs.pop(key, None)
+
+        return representation
 
     def get_last_run_at(self, instance: ExternalDataSource) -> str:
         latest_completed_run = instance.ordered_jobs[0] if instance.ordered_jobs else None  # type: ignore
@@ -429,8 +455,8 @@ def create(self, request: Request, *args: Any, **kwargs: Any) -> Response:
 
     def _handle_stripe_source(self, request: Request, *args: Any, **kwargs: Any) -> ExternalDataSource:
         payload = request.data["payload"]
-        client_secret = payload.get("client_secret")
-        account_id = payload.get("account_id", None)
+        client_secret = payload.get("stripe_secret_key")
+        account_id = payload.get("stripe_account_id", None)
         prefix = request.data.get("prefix", None)
         source_type = request.data["source_type"]
         # TODO: remove dummy vars
@@ -576,7 +602,7 @@ def _handle_sql_source(self, request: Request, *args: Any, **kwargs: Any) -> tup
 
         host = payload.get("host")
         port = payload.get("port")
-        database = payload.get("dbname")
+        database = payload.get("database")
 
         user = payload.get("user")
         password = payload.get("password")
@@ -847,7 +873,7 @@ def database_schema(self, request: Request, *arg: Any, **kwargs: Any):
 
         # Validate sourced credentials
         if source_type == ExternalDataSource.Type.STRIPE:
-            key = request.data.get("client_secret", "")
+            key = request.data.get("stripe_secret_key", "")
             if not validate_stripe_credentials(api_key=key):
                 return Response(
                     status=status.HTTP_400_BAD_REQUEST,
@@ -960,7 +986,7 @@ def database_schema(self, request: Request, *arg: Any, **kwargs: Any):
 
             host = request.data.get("host", None)
             port = request.data.get("port", None)
-            database = request.data.get("dbname", None)
+            database = request.data.get("database", None)
 
             user = request.data.get("user", None)
             password = request.data.get("password", None)