From df282f65107e2bb868d42f157f4545400c0d21e1 Mon Sep 17 00:00:00 2001 From: Tiina Turban Date: Tue, 17 Oct 2023 15:22:20 +0200 Subject: [PATCH] chore: Make pluginConfigs be keyed over config id not plugin id (#17999) --- .../src/scenes/apps/frontendAppsLogic.tsx | 4 +- frontend/src/scenes/plugins/pluginsLogic.ts | 45 ++++++++++--------- frontend/src/scenes/plugins/types.ts | 2 +- frontend/src/types.ts | 2 +- 4 files changed, 28 insertions(+), 25 deletions(-) diff --git a/frontend/src/scenes/apps/frontendAppsLogic.tsx b/frontend/src/scenes/apps/frontendAppsLogic.tsx index 1405c620eed87..e0376749c61bb 100644 --- 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([ 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 index dd03bed0a3f6b..ce98a2080f1b7 100644 --- a/frontend/src/scenes/plugins/pluginsLogic.ts +++ b/frontend/src/scenes/plugins/pluginsLogic.ts @@ -136,17 +136,6 @@ export const pluginsLogic = kea([ 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([ }, ], pluginConfigs: [ - {} as Record, + {} as Record, { loadPluginConfigs: async () => { - const pluginConfigs: Record = {} + const pluginConfigs: Record = {} 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([ // 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([ }) const newPluginConfigs: Record = { ...pluginConfigs } for (const pluginConfig of response) { - newPluginConfigs[pluginConfig.plugin] = pluginConfig + newPluginConfigs[pluginConfig.id] = pluginConfig } return newPluginConfigs }, @@ -325,7 +316,7 @@ export const pluginsLogic = kea([ const newPluginConfigs: Record = {} 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([ (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([ 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 & { id?: number } + if (pluginConfigsForPlugin) { + pluginConfig = pluginConfigsForPlugin[0] + } else { const config: Record = {} Object.entries(getConfigSchemaObject(plugin.config_schema)).forEach( ([key, { default: def }]) => { @@ -445,7 +448,7 @@ export const pluginsLogic = kea([ ) 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 index f662a93708dd5..c67dc927aac0b 100644 --- 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 & { id?: number } updateStatus: PluginUpdateStatusType hasMoved?: boolean } diff --git a/frontend/src/types.ts b/frontend/src/types.ts index 6322a3fb6c4d0..eef5a4a5fe1a5 100644 --- 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