chore: Make pluginConfigs be keyed over config id not plugin id (#17999)

PostHog · Oct 17, 2023 · df282f6 · df282f6
1 parent fd230b1
commit df282f6
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 25 deletions.
diff --git a/frontend/src/scenes/apps/frontendAppsLogic.tsx b/frontend/src/scenes/apps/frontendAppsLogic.tsx
@@ -2,7 +2,7 @@ import { actions, afterMount, connect, defaults, kea, path, reducers } from 'kea
 import type { frontendAppsLogicType } from './frontendAppsLogicType'
 import { getAppContext } from 'lib/utils/getAppContext'
 import { loaders } from 'kea-loaders'
-import { FrontendApp, FrontendAppConfig } from '~/types'
+import { FrontendApp, FrontendAppConfig, PluginConfigType } from '~/types'
 import { frontendAppRequire } from './frontendAppRequire'
 import { lemonToast } from 'lib/lemon-ui/lemonToast'
 import { pluginsLogic } from 'scenes/plugins/pluginsLogic'
@@ -31,7 +31,7 @@ export const frontendAppsLogic = kea<frontendAppsLogicType>([
             loadFrontendApp: async ({ id, pluginId, reload, attempt }) => {
                 if (!values.appConfigs[id]) {
                     if (pluginsLogic.findMounted()) {
-                        const pluginConfig = Object.values(pluginsLogic.values.pluginConfigs).find((c) => c.id === id)
+                        const pluginConfig: PluginConfigType | undefined = pluginsLogic.values.getPluginConfig(id)
                         const plugin = pluginConfig ? pluginsLogic.values.plugins[pluginConfig.plugin] : undefined
                         if (!plugin && !pluginConfig) {
                             throw Error(`Could not load metadata for app with ID ${id}`)

diff --git a/frontend/src/scenes/plugins/pluginsLogic.ts b/frontend/src/scenes/plugins/pluginsLogic.ts
@@ -136,17 +136,6 @@ export const pluginsLogic = kea<pluginsLogicType>([
                     const response = await api.create(`api/organizations/@current/plugins/${id}/upgrade`)
                     capturePluginEvent(`plugin updated`, response)
                     actions.pluginUpdated(id)
-                    // Check if we need to update the config (e.g. new required field) and if so, open the drawer.
-                    const schema = getConfigSchemaObject(response.config_schema)
-                    const pluginConfig = Object.values(values.pluginConfigs).filter((c) => c.plugin === id)[0]
-                    if (pluginConfig?.enabled) {
-                        if (
-                            Object.entries(schema).find(([key, { required }]) => required && !pluginConfig.config[key])
-                        ) {
-                            actions.editPlugin(id)
-                        }
-                    }
-
                     return { ...values.plugins, [id]: response }
                 },
                 patchPlugin: async ({ id, pluginChanges }) => {
@@ -156,14 +145,16 @@ export const pluginsLogic = kea<pluginsLogicType>([
             },
         ],
         pluginConfigs: [
-            {} as Record<string, PluginConfigType>,
+            {} as Record<number, PluginConfigType>,
             {
                 loadPluginConfigs: async () => {
-                    const pluginConfigs: Record<string, PluginConfigType> = {}
+                    const pluginConfigs: Record<number, PluginConfigType> = {}
                     const results: PluginConfigType[] = await loadPaginatedResults('api/plugin_config')
 
                     for (const pluginConfig of results) {
-                        pluginConfigs[pluginConfig.plugin] = { ...pluginConfig }
+                        if (pluginConfig.id) {
+                            pluginConfigs[pluginConfig.id] = { ...pluginConfig }
+                        }
                     }
 
                     return pluginConfigs
@@ -196,7 +187,7 @@ export const pluginsLogic = kea<pluginsLogicType>([
                         // Run the sync after we return from the loader, and save its data
                         window.setTimeout(() => response.id && actions.syncFrontendAppState(response.id), 0)
                     }
-                    return { ...pluginConfigs, [response.plugin]: response }
+                    return { ...pluginConfigs, [response.id]: response }
                 },
                 toggleEnabled: async ({ id, enabled }) => {
                     const { pluginConfigs, plugins } = values
@@ -220,7 +211,7 @@ export const pluginsLogic = kea<pluginsLogicType>([
                     })
                     const newPluginConfigs: Record<string, PluginConfigType> = { ...pluginConfigs }
                     for (const pluginConfig of response) {
-                        newPluginConfigs[pluginConfig.plugin] = pluginConfig
+                        newPluginConfigs[pluginConfig.id] = pluginConfig
                     }
                     return newPluginConfigs
                 },
@@ -325,7 +316,7 @@ export const pluginsLogic = kea<pluginsLogicType>([
                 const newPluginConfigs: Record<number, PluginConfigType> = {}
                 Object.values(pluginConfigs).forEach((pluginConfig) => {
                     if (plugins[pluginConfig.plugin]) {
-                        newPluginConfigs[pluginConfig.plugin] = pluginConfig
+                        newPluginConfigs[pluginConfig.id] = pluginConfig
                     }
                 })
                 return newPluginConfigs
@@ -420,7 +411,13 @@ export const pluginsLogic = kea<pluginsLogicType>([
             (s) => [s.pluginConfigs],
             (pluginConfigs): ((id: number) => PluginConfigType | undefined) =>
                 (id: number) =>
-                    Object.values(pluginConfigs).find(({ id: _id }) => id === _id),
+                    pluginConfigs[id],
+        ],
+        getPluginConfigsForPlugin: [
+            (s) => [s.pluginConfigs],
+            (pluginConfigs): ((pluginId: number) => PluginConfigType[]) =>
+                (pluginId: number) =>
+                    Object.values(pluginConfigs).filter((config) => config.plugin === pluginId),
         ],
         installedPlugins: [
             (s) => [s.plugins, s.pluginConfigs, s.updateStatus],
@@ -435,8 +432,14 @@ export const pluginsLogic = kea<pluginsLogicType>([
 
                 return pluginValues
                     .map((plugin, index) => {
-                        let pluginConfig: PluginConfigType = { ...pluginConfigs[plugin.id] }
-                        if (!pluginConfigs[plugin.id]) {
+                        // TODO: Currently just returning the first pluginConfig if exists or creating an empty one
+                        const pluginConfigsForPlugin = Object.values(pluginConfigs).filter(
+                            (config) => config.plugin === plugin.id
+                        )
+                        let pluginConfig: Omit<PluginConfigType, 'id'> & { id?: number }
+                        if (pluginConfigsForPlugin) {
+                            pluginConfig = pluginConfigsForPlugin[0]
+                        } else {
                             const config: Record<string, any> = {}
                             Object.entries(getConfigSchemaObject(plugin.config_schema)).forEach(
                                 ([key, { default: def }]) => {
@@ -445,7 +448,7 @@ export const pluginsLogic = kea<pluginsLogicType>([
                             )
 
                             pluginConfig = {
-                                id: undefined,
+                                id: undefined, // TODO: only place where we use undefined for id
                                 team_id: currentTeam.id,
                                 plugin: plugin.id,
                                 enabled: false,

diff --git a/frontend/src/scenes/plugins/types.ts b/frontend/src/scenes/plugins/types.ts
@@ -17,7 +17,7 @@ export enum PluginRepositoryEntryType {
 }
 
 export interface PluginTypeWithConfig extends PluginType {
-    pluginConfig: PluginConfigType
+    pluginConfig: Omit<PluginConfigType, 'id'> & { id?: number }
     updateStatus: PluginUpdateStatusType
     hasMoved?: boolean
 }

diff --git a/frontend/src/types.ts b/frontend/src/types.ts
@@ -1459,7 +1459,7 @@ export interface JobSpec {
 }
 
 export interface PluginConfigType {
-    id?: number
+    id: number
     plugin: number
     team_id: number
     enabled: boolean