From 6813f758bfd9db43024cffae9d8eb64bcdb07d48 Mon Sep 17 00:00:00 2001 From: Nikita Karetnikov Date: Thu, 29 Feb 2024 17:55:00 +0100 Subject: [PATCH] Allow passing `variables` via raw YAML (#354) * Allow passing `variables` via raw YAML Fixes #350. * Replace `formatCode` with `stringify` * Use `variables` from the global state * Do not repeat `variables` twice * Get back to using `formatCode` The custom pretty-printer is there to avoid printing `[]` when no data is available, which is not very user-friendly. * Rename `variables` to `environmentVariables` * Check if text input is working in playwright tests * Fixup: serialize as `variables` * Add tests * Add more tests * Fixup: serialize as `variables` on create * Update tests * Fixup: serialize as `variables` on edit * Fixup: serialize as `variables` on edit * Rename back to `variables` in `CondaSpecification` * Fixup: rename back to `variables` to avoid serialization bugs * Fix the test * Revert "Check if text input is working in playwright tests" This reverts commit 5f084355e54757639a83b2374dec5f49a9a18f17. * Update tests * Create a new test * Check siblings * Add waitFor * Run linter * Save variables on toggle on create env * Save variables on toggle on edit env * Revert all test changes * Remove redundant dispatch * Fix a broken test --- src/common/models/CondaSpecification.ts | 1 + .../Specification/SpecificationCreate.tsx | 57 ++++++++++++---- .../environmentCreateSlice.ts | 12 +++- .../Specification/SpecificationEdit.tsx | 66 ++++++++++++++++--- .../environmentVariablesSlice.ts | 40 +++++++++++ src/features/environmentVariables/index.tsx | 1 + src/features/yamlEditor/components/editor.tsx | 1 + src/store/rootReducer.ts | 2 + .../selectors/condaSpecificationSelector.ts | 4 +- .../SpecificationCreate.test.tsx | 3 +- 10 files changed, 161 insertions(+), 26 deletions(-) create mode 100644 src/features/environmentVariables/environmentVariablesSlice.ts create mode 100644 src/features/environmentVariables/index.tsx diff --git a/src/common/models/CondaSpecification.ts b/src/common/models/CondaSpecification.ts index 24bd6e78..0512aeb0 100644 --- a/src/common/models/CondaSpecification.ts +++ b/src/common/models/CondaSpecification.ts @@ -4,5 +4,6 @@ export type CondaSpecification = { name: string; channels: string[]; dependencies: (string | CondaSpecificationPip)[]; + variables: Record; prefix?: string | null; }; diff --git a/src/features/environmentCreate/components/Specification/SpecificationCreate.tsx b/src/features/environmentCreate/components/Specification/SpecificationCreate.tsx index 0c895edd..7ca76b74 100644 --- a/src/features/environmentCreate/components/Specification/SpecificationCreate.tsx +++ b/src/features/environmentCreate/components/Specification/SpecificationCreate.tsx @@ -16,14 +16,15 @@ import { getStylesForStyleType } from "../../../../utils/helpers"; export const SpecificationCreate = ({ onCreateEnvironment }: any) => { const dispatch = useAppDispatch(); - const { channels, requestedPackages } = useAppSelector( + const { channels, requestedPackages, environmentVariables } = useAppSelector( state => state.environmentCreate ); const [show, setShow] = useState(false); const [editorContent, setEditorContent] = useState<{ channels: string[]; dependencies: string[]; - }>({ channels: [], dependencies: [] }); + variables: Record; + }>({ channels: [], dependencies: [], variables: {} }); const buttonStyles = getStylesForStyleType( { padding: "5px 60px" }, @@ -36,21 +37,37 @@ export const SpecificationCreate = ({ onCreateEnvironment }: any) => { const onUpdateEditor = ({ channels, - dependencies + dependencies, + variables }: { channels: string[]; dependencies: string[]; + variables: Record; }) => { - const code = { channels, dependencies }; + const code = { channels, dependencies, variables }; - if (!channels || channels.length === 0) { + // Note: the [null] checks are due to empty lists in the pretty-printed case + // of formatCode + if ( + !channels || + channels.length === 0 || + (channels.length === 1 && channels[0] === null) + ) { code.channels = []; } - if (!dependencies || dependencies.length === 0) { + if ( + !dependencies || + dependencies.length === 0 || + (dependencies.length === 1 && dependencies[0] === null) + ) { code.dependencies = []; } + if (!variables || Object.keys(variables).length === 0) { + code.variables = {}; + } + setEditorContent(code); }; @@ -59,27 +76,41 @@ export const SpecificationCreate = ({ onCreateEnvironment }: any) => { dispatch( editorCodeUpdated({ channels: editorContent.channels, - dependencies: editorContent.dependencies + dependencies: editorContent.dependencies, + variables: editorContent.variables }) ); } else { - setEditorContent({ dependencies: requestedPackages, channels }); + setEditorContent({ + dependencies: requestedPackages, + variables: environmentVariables, + channels + }); } setShow(value); }; - const formatCode = (channels: string[], dependencies: string[]) => { + const formatCode = ( + channels: string[], + dependencies: string[], + variables: Record + ) => { if (channels.length === 0 && dependencies.length === 0) { - return "channels:\n -\ndependencies:\n -"; + // Note: these empty pretty-printed lists translate to [null] + return "channels:\n -\ndependencies:\n -\n" + stringify({ variables }); } - return stringify({ channels, dependencies: dependencies }); + return stringify({ channels, dependencies, variables }); }; const handleSubmit = () => { const code = show ? editorContent - : { dependencies: requestedPackages, channels }; + : { + dependencies: requestedPackages, + variables: environmentVariables, + channels + }; onCreateEnvironment(code); }; @@ -99,7 +130,7 @@ export const SpecificationCreate = ({ onCreateEnvironment }: any) => { {show ? ( ) : ( diff --git a/src/features/environmentCreate/environmentCreateSlice.ts b/src/features/environmentCreate/environmentCreateSlice.ts index bbb10828..6a8e0201 100644 --- a/src/features/environmentCreate/environmentCreateSlice.ts +++ b/src/features/environmentCreate/environmentCreateSlice.ts @@ -5,13 +5,15 @@ export interface IEnvironmentCreateState { description: string; requestedPackages: string[]; channels: string[]; + environmentVariables: Record; } const initialState: IEnvironmentCreateState = { name: "", description: "", + requestedPackages: [], channels: [], - requestedPackages: [] + environmentVariables: {} }; export const environmentCreateSlice = createSlice({ @@ -47,16 +49,22 @@ export const environmentCreateSlice = createSlice({ }, editorCodeUpdated: ( state, - action: PayloadAction<{ dependencies: string[]; channels: string[] }> + action: PayloadAction<{ + dependencies: string[]; + channels: string[]; + variables: Record; + }> ) => { state.requestedPackages = action.payload.dependencies; state.channels = action.payload.channels; + state.environmentVariables = action.payload.variables; }, environmentCreateStateCleared: state => { state.name = ""; state.description = ""; state.channels = []; state.requestedPackages = []; + state.environmentVariables = {}; } } }); diff --git a/src/features/environmentDetails/components/Specification/SpecificationEdit.tsx b/src/features/environmentDetails/components/Specification/SpecificationEdit.tsx index 49c4db35..f4afd065 100644 --- a/src/features/environmentDetails/components/Specification/SpecificationEdit.tsx +++ b/src/features/environmentDetails/components/Specification/SpecificationEdit.tsx @@ -11,6 +11,7 @@ import { cloneDeep, debounce } from "lodash"; import { stringify } from "yaml"; import { BlockContainerEditMode } from "../../../../components"; import { ChannelsEdit, updateChannels } from "../../../../features/channels"; +import { updateEnvironmentVariables } from "../../../../features/environmentVariables"; import { Dependencies, pageChanged } from "../../../../features/dependencies"; import { modeChanged, @@ -44,6 +45,9 @@ export const SpecificationEdit = ({ const { requestedPackages } = useAppSelector( state => state.requestedPackages ); + const { environmentVariables } = useAppSelector( + state => state.environmentVariables + ); const { dependencies, size, count, page } = useAppSelector( state => state.dependencies ); @@ -54,11 +58,17 @@ export const SpecificationEdit = ({ const [code, setCode] = useState<{ dependencies: (string | CondaSpecificationPip)[]; channels: string[]; - }>({ dependencies: requestedPackages, channels }); + variables: Record; + }>({ + dependencies: requestedPackages, + variables: environmentVariables, + channels + }); const [envIsUpdated, setEnvIsUpdated] = useState(false); const initialChannels = useRef(cloneDeep(channels)); const initialPackages = useRef(cloneDeep(requestedPackages)); + const initialEnvironmentVariables = useRef(cloneDeep(environmentVariables)); const stringifiedInitialChannels = useMemo(() => { return JSON.stringify(initialChannels.current); @@ -68,6 +78,10 @@ export const SpecificationEdit = ({ return JSON.stringify(initialPackages.current); }, [initialPackages.current]); + const stringifiedInitialEnvironmentVariables = useMemo(() => { + return JSON.stringify(initialEnvironmentVariables.current); + }, [initialEnvironmentVariables.current]); + const onUpdateChannels = useCallback((channels: string[]) => { dispatch(updateChannels(channels)); onDefaultEnvIsChanged(false); @@ -81,16 +95,21 @@ export const SpecificationEdit = ({ const onUpdateEditor = debounce( ({ channels, - dependencies + dependencies, + variables }: { channels: string[]; dependencies: string[]; + variables: Record; }) => { - const code = { dependencies, channels }; + const code = { dependencies, channels, variables }; const isDifferentChannels = JSON.stringify(code.channels) !== stringifiedInitialChannels; const isDifferentPackages = JSON.stringify(code.dependencies) !== stringifiedInitialPackages; + const isDifferentEnvironmentVariables = + JSON.stringify(code.variables) !== + stringifiedInitialEnvironmentVariables; if (!channels || channels.length === 0) { code.channels = []; @@ -100,7 +119,15 @@ export const SpecificationEdit = ({ code.dependencies = []; } - if (isDifferentChannels || isDifferentPackages) { + if (!variables || Object.keys(variables).length === 0) { + code.variables = {}; + } + + if ( + isDifferentChannels || + isDifferentPackages || + isDifferentEnvironmentVariables + ) { setEnvIsUpdated(true); onUpdateDefaultEnvironment(false); onSpecificationIsChanged(true); @@ -115,8 +142,13 @@ export const SpecificationEdit = ({ if (show) { dispatch(updatePackages(code.dependencies)); dispatch(updateChannels(code.channels)); + dispatch(updateEnvironmentVariables(code.variables)); } else { - setCode({ dependencies: requestedPackages, channels }); + setCode({ + dependencies: requestedPackages, + variables: environmentVariables, + channels + }); } setShow(value); @@ -125,7 +157,11 @@ export const SpecificationEdit = ({ const onEditEnvironment = () => { const envContent = show ? code - : { dependencies: requestedPackages, channels }; + : { + dependencies: requestedPackages, + variables: environmentVariables, + channels + }; onUpdateEnvironment(envContent); }; @@ -136,6 +172,7 @@ export const SpecificationEdit = ({ dispatch(modeChanged(EnvironmentDetailsModes.READ)); dispatch(updatePackages(initialPackages.current)); dispatch(updateChannels(initialChannels.current)); + dispatch(updateEnvironmentVariables(initialEnvironmentVariables.current)); }; useEffect(() => { @@ -147,13 +184,20 @@ export const SpecificationEdit = ({ JSON.stringify(channels) !== stringifiedInitialChannels; const isDifferentPackages = JSON.stringify(requestedPackages) !== stringifiedInitialPackages; + const isDifferentEnvironmentVariables = + JSON.stringify(environmentVariables) !== + stringifiedInitialEnvironmentVariables; if (defaultEnvVersIsChanged) { setEnvIsUpdated(false); - } else if (isDifferentChannels || isDifferentPackages) { + } else if ( + isDifferentChannels || + isDifferentPackages || + isDifferentEnvironmentVariables + ) { setEnvIsUpdated(true); } - }, [channels, requestedPackages, descriptionUpdated]); + }, [channels, requestedPackages, environmentVariables, descriptionUpdated]); return ( {show ? ( ) : ( diff --git a/src/features/environmentVariables/environmentVariablesSlice.ts b/src/features/environmentVariables/environmentVariablesSlice.ts new file mode 100644 index 00000000..5c040ffa --- /dev/null +++ b/src/features/environmentVariables/environmentVariablesSlice.ts @@ -0,0 +1,40 @@ +import { createSlice } from "@reduxjs/toolkit"; +import { environmentDetailsApiSlice } from "../environmentDetails"; + +export interface IEnvironmentVariablesState { + environmentVariables: Record; +} + +const initialState: IEnvironmentVariablesState = { environmentVariables: {} }; + +export const environmentVariablesSlice = createSlice({ + name: "environmentVariables", + initialState, + reducers: { + updateEnvironmentVariables: (state, action) => { + const environmentVariables = action.payload; + state.environmentVariables = environmentVariables; + } + }, + extraReducers: builder => { + builder.addMatcher( + environmentDetailsApiSlice.endpoints.getBuild.matchFulfilled, + ( + state, + { + payload: { + data: { + specification: { + spec: { variables: environmentVariables } + } + } + } + } + ) => { + state.environmentVariables = environmentVariables; + } + ); + } +}); + +export const { updateEnvironmentVariables } = environmentVariablesSlice.actions; diff --git a/src/features/environmentVariables/index.tsx b/src/features/environmentVariables/index.tsx new file mode 100644 index 00000000..b4a52f62 --- /dev/null +++ b/src/features/environmentVariables/index.tsx @@ -0,0 +1 @@ +export * from "./environmentVariablesSlice"; diff --git a/src/features/yamlEditor/components/editor.tsx b/src/features/yamlEditor/components/editor.tsx index a9ee6722..77f015cd 100644 --- a/src/features/yamlEditor/components/editor.tsx +++ b/src/features/yamlEditor/components/editor.tsx @@ -12,6 +12,7 @@ export interface ICodeEditor { onChangeEditor: (code: { channels: string[]; dependencies: string[]; + variables: Record; }) => void; } diff --git a/src/store/rootReducer.ts b/src/store/rootReducer.ts index f36102a6..d0a427ef 100644 --- a/src/store/rootReducer.ts +++ b/src/store/rootReducer.ts @@ -3,6 +3,7 @@ import { channelsSlice } from "../features/channels"; import { dependenciesSlice } from "../features/dependencies"; import { environmentDetailsSlice } from "../features/environmentDetails"; import { requestedPackagesSlice } from "../features/requestedPackages"; +import { environmentVariablesSlice } from "../features/environmentVariables"; import { tabsSlice } from "../features/tabs"; import { enviromentsSlice } from "../features/metadata"; import { environmentCreateSlice } from "../features/environmentCreate/environmentCreateSlice"; @@ -11,6 +12,7 @@ export const rootReducer = { [apiSlice.reducerPath]: apiSlice.reducer, channels: channelsSlice.reducer, requestedPackages: requestedPackagesSlice.reducer, + environmentVariables: environmentVariablesSlice.reducer, tabs: tabsSlice.reducer, enviroments: enviromentsSlice.reducer, environmentDetails: environmentDetailsSlice.reducer, diff --git a/src/store/selectors/condaSpecificationSelector.ts b/src/store/selectors/condaSpecificationSelector.ts index d001c0d1..bdb50d08 100644 --- a/src/store/selectors/condaSpecificationSelector.ts +++ b/src/store/selectors/condaSpecificationSelector.ts @@ -7,6 +7,7 @@ export const selectCondaSpecification = ( const { channels: { channels }, requestedPackages: { requestedPackages }, + environmentVariables: { environmentVariables }, environmentDetails: { name, prefix } } = state; @@ -14,7 +15,8 @@ export const selectCondaSpecification = ( channels, dependencies: requestedPackages, name, - prefix + prefix, + variables: environmentVariables }; return condaSpecification; diff --git a/test/environmentCreate/SpecificationCreate.test.tsx b/test/environmentCreate/SpecificationCreate.test.tsx index 1e9fefd1..a0b92cd5 100644 --- a/test/environmentCreate/SpecificationCreate.test.tsx +++ b/test/environmentCreate/SpecificationCreate.test.tsx @@ -51,7 +51,8 @@ describe("", () => { fireEvent.click(createButton); expect(mockOnCreateEnvironment).toHaveBeenCalledWith({ channels: [], - dependencies: [] + dependencies: [], + variables: {} }); });