Skip to content

Commit

Permalink
Allow passing variables via raw YAML (conda-incubator#354)
Browse files Browse the repository at this point in the history
* Allow passing `variables` via raw YAML

Fixes conda-incubator#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 5f08435.

* 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
  • Loading branch information
Nikita Karetnikov authored and gabalafou committed Jul 9, 2024
1 parent 8e10e65 commit 0608f27
Show file tree
Hide file tree
Showing 10 changed files with 161 additions and 26 deletions.
1 change: 1 addition & 0 deletions src/common/models/CondaSpecification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@ export type CondaSpecification = {
name: string;
channels: string[];
dependencies: (string | CondaSpecificationPip)[];
variables: Record<string, string>;
prefix?: string | null;
};
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string>;
}>({ channels: [], dependencies: [], variables: {} });

const buttonStyles = getStylesForStyleType(
{ padding: "5px 60px" },
Expand All @@ -36,21 +37,37 @@ export const SpecificationCreate = ({ onCreateEnvironment }: any) => {

const onUpdateEditor = ({
channels,
dependencies
dependencies,
variables
}: {
channels: string[];
dependencies: string[];
variables: Record<string, string>;
}) => {
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);
};

Expand All @@ -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<string, string>
) => {
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);
};
Expand All @@ -99,7 +130,7 @@ export const SpecificationCreate = ({ onCreateEnvironment }: any) => {
<Box>
{show ? (
<CodeEditor
code={formatCode(channels, requestedPackages)}
code={formatCode(channels, requestedPackages, environmentVariables)}
onChangeEditor={onUpdateEditor}
/>
) : (
Expand Down
12 changes: 10 additions & 2 deletions src/features/environmentCreate/environmentCreateSlice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ export interface IEnvironmentCreateState {
description: string;
requestedPackages: string[];
channels: string[];
environmentVariables: Record<string, string>;
}

const initialState: IEnvironmentCreateState = {
name: "",
description: "",
requestedPackages: [],
channels: [],
requestedPackages: []
environmentVariables: {}
};

export const environmentCreateSlice = createSlice({
Expand Down Expand Up @@ -47,16 +49,22 @@ export const environmentCreateSlice = createSlice({
},
editorCodeUpdated: (
state,
action: PayloadAction<{ dependencies: string[]; channels: string[] }>
action: PayloadAction<{
dependencies: string[];
channels: string[];
variables: Record<string, string>;
}>
) => {
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 = {};
}
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
);
Expand All @@ -54,11 +58,17 @@ export const SpecificationEdit = ({
const [code, setCode] = useState<{
dependencies: (string | CondaSpecificationPip)[];
channels: string[];
}>({ dependencies: requestedPackages, channels });
variables: Record<string, string>;
}>({
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);
Expand All @@ -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);
Expand All @@ -81,16 +95,21 @@ export const SpecificationEdit = ({
const onUpdateEditor = debounce(
({
channels,
dependencies
dependencies,
variables
}: {
channels: string[];
dependencies: string[];
variables: Record<string, string>;
}) => {
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 = [];
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -125,7 +157,11 @@ export const SpecificationEdit = ({
const onEditEnvironment = () => {
const envContent = show
? code
: { dependencies: requestedPackages, channels };
: {
dependencies: requestedPackages,
variables: environmentVariables,
channels
};

onUpdateEnvironment(envContent);
};
Expand All @@ -136,6 +172,7 @@ export const SpecificationEdit = ({
dispatch(modeChanged(EnvironmentDetailsModes.READ));
dispatch(updatePackages(initialPackages.current));
dispatch(updateChannels(initialChannels.current));
dispatch(updateEnvironmentVariables(initialEnvironmentVariables.current));
};

useEffect(() => {
Expand All @@ -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 (
<BlockContainerEditMode
Expand All @@ -164,7 +208,11 @@ export const SpecificationEdit = ({
<Box>
{show ? (
<CodeEditor
code={stringify({ channels, dependencies: requestedPackages })}
code={stringify({
channels,
dependencies: requestedPackages,
variables: environmentVariables
})}
onChangeEditor={onUpdateEditor}
/>
) : (
Expand Down
40 changes: 40 additions & 0 deletions src/features/environmentVariables/environmentVariablesSlice.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { createSlice } from "@reduxjs/toolkit";
import { environmentDetailsApiSlice } from "../environmentDetails";

export interface IEnvironmentVariablesState {
environmentVariables: Record<string, string>;
}

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;
1 change: 1 addition & 0 deletions src/features/environmentVariables/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./environmentVariablesSlice";
1 change: 1 addition & 0 deletions src/features/yamlEditor/components/editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export interface ICodeEditor {
onChangeEditor: (code: {
channels: string[];
dependencies: string[];
variables: Record<string, string>;
}) => void;
}

Expand Down
2 changes: 2 additions & 0 deletions src/store/rootReducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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,
Expand Down
Loading

0 comments on commit 0608f27

Please sign in to comment.