From 488393007f3c0fd60c1e03d23abba9835f48dc16 Mon Sep 17 00:00:00 2001
From: Ali <83188384+testA113@users.noreply.github.com>
Date: Wed, 3 Jan 2024 08:17:54 +1300
Subject: [PATCH] refactor(app): migrate env var form section [EE-6232]
(#10499)
* refactor(app): migrate env var form section [EE-6232]
* allow undoing delete in inputlist
---
app/kubernetes/helpers/application/index.js | 16 +--
.../models/application/formValues.js | 11 +-
app/kubernetes/react/components/index.ts | 12 ++
.../create/createApplication.html | 103 +-----------------
.../create/createApplicationController.js | 33 ++----
app/portainer/react/components/index.ts | 2 +-
.../EnvironmentVariableItem.tsx | 59 ++++++++++
.../EnvironmentVariablesFieldset.tsx | 18 ++-
.../EnvironmentVariablesPanel.tsx | 12 +-
.../SimpleMode.tsx | 66 ++---------
.../EnvironmentVariablesFieldset/types.ts | 1 +
.../form-components/Input/InputLabeled.tsx | 13 ++-
.../InputList/InputList.stories.tsx | 17 ++-
.../form-components/InputList/InputList.tsx | 83 ++++++++++++--
.../form-components/InputList/utils.ts | 11 ++
.../kubeEnvVarValidationSchema.ts | 26 +++++
16 files changed, 274 insertions(+), 209 deletions(-)
create mode 100644 app/react/components/form-components/EnvironmentVariablesFieldset/EnvironmentVariableItem.tsx
create mode 100644 app/react/kubernetes/applications/ApplicationForm/kubeEnvVarValidationSchema.ts
diff --git a/app/kubernetes/helpers/application/index.js b/app/kubernetes/helpers/application/index.js
index 89e699a63..21ff38277 100644
--- a/app/kubernetes/helpers/application/index.js
+++ b/app/kubernetes/helpers/application/index.js
@@ -108,14 +108,14 @@ class KubernetesApplicationHelper {
/* #region ENV VARIABLES FV <> ENV */
static generateEnvFromEnvVariables(envVariables) {
- _.remove(envVariables, (item) => item.NeedsDeletion);
+ _.remove(envVariables, (item) => item.needsDeletion);
const env = _.map(envVariables, (item) => {
const res = new KubernetesApplicationEnvPayload();
- res.name = item.Name;
- if (item.Value === undefined) {
+ res.name = item.name;
+ if (item.value === undefined) {
delete res.value;
} else {
- res.value = item.Value;
+ res.value = item.value;
}
return res;
});
@@ -128,10 +128,10 @@ class KubernetesApplicationHelper {
return;
}
const res = new KubernetesApplicationEnvironmentVariableFormValue();
- res.Name = item.name;
- res.Value = item.value;
- res.IsNew = false;
- res.NameIndex = item.name;
+ res.name = item.name;
+ res.value = item.value;
+ res.isNew = false;
+ res.nameIndex = item.name;
return res;
});
return _.without(envVariables, undefined);
diff --git a/app/kubernetes/models/application/formValues.js b/app/kubernetes/models/application/formValues.js
index ce3415711..f4add318b 100644
--- a/app/kubernetes/models/application/formValues.js
+++ b/app/kubernetes/models/application/formValues.js
@@ -70,12 +70,11 @@ export class KubernetesApplicationConfigurationFormValue {
* KubernetesApplicationEnvironmentVariableFormValue Model
*/
const _KubernetesApplicationEnvironmentVariableFormValue = Object.freeze({
- Name: '',
- Value: '',
- IsSecret: false,
- NeedsDeletion: false,
- IsNew: true,
- NameIndex: '', // keep the original name for sorting
+ name: '',
+ value: '',
+ needsDeletion: false,
+ isNew: true,
+ nameIndex: '', // keep the original name for sorting
});
export class KubernetesApplicationEnvironmentVariableFormValue {
diff --git a/app/kubernetes/react/components/index.ts b/app/kubernetes/react/components/index.ts
index c433c0770..16e531a75 100644
--- a/app/kubernetes/react/components/index.ts
+++ b/app/kubernetes/react/components/index.ts
@@ -24,6 +24,9 @@ import { YAMLInspector } from '@/react/kubernetes/components/YAMLInspector';
import { ApplicationsStacksDatatable } from '@/react/kubernetes/applications/ListView/ApplicationsStacksDatatable';
import { NodesDatatable } from '@/react/kubernetes/cluster/HomeView/NodesDatatable';
import { StackName } from '@/react/kubernetes/DeployView/StackName/StackName';
+import { kubeEnvVarValidationSchema } from '@/react/kubernetes/applications/ApplicationForm/kubeEnvVarValidationSchema';
+
+import { EnvironmentVariablesFieldset } from '@@/form-components/EnvironmentVariablesFieldset';
export const ngModule = angular
.module('portainer.kubernetes.react.components', [])
@@ -174,3 +177,12 @@ withFormValidation(
['values', 'onChange', 'appName', 'selector', 'isEditMode', 'namespace'],
kubeServicesValidation
);
+
+withFormValidation(
+ ngModule,
+ EnvironmentVariablesFieldset,
+ 'kubeEnvironmentVariablesFieldset',
+ ['canUndoDelete'],
+ // use kubeEnvVarValidationSchema instead of envVarValidation to add a regex matches rule
+ kubeEnvVarValidationSchema
+);
diff --git a/app/kubernetes/views/applications/create/createApplication.html b/app/kubernetes/views/applications/create/createApplication.html
index 625eecb3f..56c9bc261 100644
--- a/app/kubernetes/views/applications/create/createApplication.html
+++ b/app/kubernetes/views/applications/create/createApplication.html
@@ -383,103 +383,12 @@
Environment variables
-
-
-
-
-
- Add environment variable
-
+
+
diff --git a/app/kubernetes/views/applications/create/createApplicationController.js b/app/kubernetes/views/applications/create/createApplicationController.js
index 884f85f35..7fd07e967 100644
--- a/app/kubernetes/views/applications/create/createApplicationController.js
+++ b/app/kubernetes/views/applications/create/createApplicationController.js
@@ -154,6 +154,7 @@ class KubernetesCreateApplicationController {
this.supportGlobalDeployment = this.supportGlobalDeployment.bind(this);
this.onChangePlacementType = this.onChangePlacementType.bind(this);
this.onServicesChange = this.onServicesChange.bind(this);
+ this.onEnvironmentVariableChange = this.onEnvironmentVariableChange.bind(this);
}
/* #endregion */
@@ -359,30 +360,14 @@ class KubernetesCreateApplicationController {
/* #endregion */
/* #region ENVIRONMENT UI MANAGEMENT */
- addEnvironmentVariable() {
- this.formValues.EnvironmentVariables.push(new KubernetesApplicationEnvironmentVariableFormValue());
- }
-
- restoreEnvironmentVariable(item) {
- item.NeedsDeletion = false;
- }
-
- removeEnvironmentVariable(item) {
- const index = this.formValues.EnvironmentVariables.indexOf(item);
- if (index !== -1) {
- const envVar = this.formValues.EnvironmentVariables[index];
- if (!envVar.IsNew) {
- envVar.NeedsDeletion = true;
- } else {
- this.formValues.EnvironmentVariables.splice(index, 1);
- }
- }
- this.onChangeEnvironmentName();
- }
-
- onChangeEnvironmentName() {
- this.state.duplicates.environmentVariables.refs = KubernetesFormValidationHelper.getDuplicates(_.map(this.formValues.EnvironmentVariables, 'Name'));
- this.state.duplicates.environmentVariables.hasRefs = Object.keys(this.state.duplicates.environmentVariables.refs).length > 0;
+ onEnvironmentVariableChange(enviromnentVariables) {
+ return this.$async(async () => {
+ const newEnvVars = enviromnentVariables.map((envVar) => {
+ const newEnvVar = new KubernetesApplicationEnvironmentVariableFormValue();
+ return { newEnvVar, ...envVar };
+ });
+ this.formValues.EnvironmentVariables = newEnvVars;
+ });
}
/* #endregion */
diff --git a/app/portainer/react/components/index.ts b/app/portainer/react/components/index.ts
index 2730fc890..5e1eb2920 100644
--- a/app/portainer/react/components/index.ts
+++ b/app/portainer/react/components/index.ts
@@ -242,7 +242,7 @@ withFormValidation(
ngModule,
EnvironmentVariablesFieldset,
'environmentVariablesFieldset',
- [],
+ ['canUndoDelete'],
envVarValidation
);
diff --git a/app/react/components/form-components/EnvironmentVariablesFieldset/EnvironmentVariableItem.tsx b/app/react/components/form-components/EnvironmentVariablesFieldset/EnvironmentVariableItem.tsx
new file mode 100644
index 000000000..9fe71212f
--- /dev/null
+++ b/app/react/components/form-components/EnvironmentVariablesFieldset/EnvironmentVariableItem.tsx
@@ -0,0 +1,59 @@
+import { FormError } from '../FormError';
+import { InputLabeled } from '../Input/InputLabeled';
+import { ItemProps } from '../InputList';
+
+import { EnvVar } from './types';
+
+export function EnvironmentVariableItem({
+ item,
+ onChange,
+ disabled,
+ error,
+ readOnly,
+ index,
+}: ItemProps) {
+ return (
+
+
+
+
handleChange({ name: e.target.value })}
+ disabled={disabled}
+ needsDeletion={item.needsDeletion}
+ readOnly={readOnly}
+ placeholder="e.g. FOO"
+ size="small"
+ id={`env-name${index}`}
+ />
+ {error && (
+
+
+ {Object.values(error)[0]}
+
+
+ )}
+
+
handleChange({ value: e.target.value })}
+ disabled={disabled}
+ needsDeletion={item.needsDeletion}
+ readOnly={readOnly}
+ placeholder="e.g. bar"
+ size="small"
+ id={`env-value${index}`}
+ />
+
+
+ );
+
+ function handleChange(partial: Partial) {
+ onChange({ ...item, ...partial });
+ }
+}
diff --git a/app/react/components/form-components/EnvironmentVariablesFieldset/EnvironmentVariablesFieldset.tsx b/app/react/components/form-components/EnvironmentVariablesFieldset/EnvironmentVariablesFieldset.tsx
index 7596482be..0c8ef23da 100644
--- a/app/react/components/form-components/EnvironmentVariablesFieldset/EnvironmentVariablesFieldset.tsx
+++ b/app/react/components/form-components/EnvironmentVariablesFieldset/EnvironmentVariablesFieldset.tsx
@@ -1,7 +1,8 @@
import { useState } from 'react';
-import { array, object, SchemaOf, string } from 'yup';
+import { array, boolean, object, SchemaOf, string } from 'yup';
import { ArrayError } from '../InputList/InputList';
+import { buildUniquenessTest } from '../validate-unique';
import { AdvancedMode } from './AdvancedMode';
import { SimpleMode } from './SimpleMode';
@@ -11,21 +12,24 @@ export function EnvironmentVariablesFieldset({
onChange,
values,
errors,
+ canUndoDelete,
}: {
values: Value;
onChange(value: Value): void;
errors?: ArrayError;
+ canUndoDelete?: boolean;
}) {
const [simpleMode, setSimpleMode] = useState(true);
return (
-
+ <>
{simpleMode ? (
setSimpleMode(false)}
onChange={onChange}
value={values}
errors={errors}
+ canUndoDelete={canUndoDelete}
/>
) : (
)}
-
+ >
);
}
@@ -43,6 +47,14 @@ export function envVarValidation(): SchemaOf {
object({
name: string().required('Name is required'),
value: string().default(''),
+ needsDeletion: boolean().default(false),
})
+ ).test(
+ 'unique',
+ 'This environment variable is already defined.',
+ buildUniquenessTest(
+ () => 'This environment variable is already defined.',
+ 'name'
+ )
);
}
diff --git a/app/react/components/form-components/EnvironmentVariablesFieldset/EnvironmentVariablesPanel.tsx b/app/react/components/form-components/EnvironmentVariablesFieldset/EnvironmentVariablesPanel.tsx
index 05a844e91..bc8ad5cb9 100644
--- a/app/react/components/form-components/EnvironmentVariablesFieldset/EnvironmentVariablesPanel.tsx
+++ b/app/react/components/form-components/EnvironmentVariablesFieldset/EnvironmentVariablesPanel.tsx
@@ -34,11 +34,13 @@ export function EnvironmentVariablesPanel({
)}
-
+
+
+
{showHelpMessage && (
diff --git a/app/react/components/form-components/EnvironmentVariablesFieldset/SimpleMode.tsx b/app/react/components/form-components/EnvironmentVariablesFieldset/SimpleMode.tsx
index 852c27700..ef9f155ae 100644
--- a/app/react/components/form-components/EnvironmentVariablesFieldset/SimpleMode.tsx
+++ b/app/react/components/form-components/EnvironmentVariablesFieldset/SimpleMode.tsx
@@ -7,24 +7,24 @@ import { Button } from '@@/buttons';
import { TextTip } from '@@/Tip/TextTip';
import { FileUploadField } from '@@/form-components/FileUpload';
import { InputList } from '@@/form-components/InputList';
-import { ArrayError, ItemProps } from '@@/form-components/InputList/InputList';
-import { InputLabeled } from '@@/form-components/Input/InputLabeled';
+import { ArrayError } from '@@/form-components/InputList/InputList';
-import { FormError } from '../FormError';
-
-import { type EnvVar, type Value } from './types';
+import type { Value } from './types';
import { parseDotEnvFile } from './utils';
+import { EnvironmentVariableItem } from './EnvironmentVariableItem';
export function SimpleMode({
value,
onChange,
onAdvancedModeClick,
errors,
+ canUndoDelete,
}: {
value: Value;
onChange: (value: Value) => void;
onAdvancedModeClick: () => void;
errors?: ArrayError
;
+ canUndoDelete?: boolean;
}) {
return (
<>
@@ -47,13 +47,17 @@ export function SimpleMode({
onChange={onChange}
value={value}
isAddButtonHidden
- item={Item}
+ item={EnvironmentVariableItem}
errors={errors}
+ canUndoDelete={canUndoDelete}
/>
onChange([...value, { name: '', value: '' }])}
+ onClick={() =>
+ onChange([...value, { name: '', value: '', needsDeletion: false }])
+ }
+ className="!ml-0"
color="default"
icon={Plus}
>
@@ -66,54 +70,6 @@ export function SimpleMode({
);
}
-function Item({
- item,
- onChange,
- disabled,
- error,
- readOnly,
- index,
-}: ItemProps) {
- return (
-
-
- handleChange({ name: e.target.value })}
- disabled={disabled}
- readOnly={readOnly}
- placeholder="e.g. FOO"
- size="small"
- id={`env-name${index}`}
- />
- handleChange({ value: e.target.value })}
- disabled={disabled}
- readOnly={readOnly}
- placeholder="e.g. bar"
- size="small"
- id={`env-value${index}`}
- />
-
-
- {!!error && (
-
- {Object.values(error)[0]}
-
- )}
-
- );
-
- function handleChange(partial: Partial) {
- onChange({ ...item, ...partial });
- }
-}
-
function FileEnv({ onChooseFile }: { onChooseFile: (file: Value) => void }) {
const [file, setFile] = useState(null);
diff --git a/app/react/components/form-components/EnvironmentVariablesFieldset/types.ts b/app/react/components/form-components/EnvironmentVariablesFieldset/types.ts
index 5366256db..bc57e1934 100644
--- a/app/react/components/form-components/EnvironmentVariablesFieldset/types.ts
+++ b/app/react/components/form-components/EnvironmentVariablesFieldset/types.ts
@@ -1,6 +1,7 @@
export interface EnvVar {
name: string;
value?: string;
+ needsDeletion?: boolean;
}
export type Value = Array;
diff --git a/app/react/components/form-components/Input/InputLabeled.tsx b/app/react/components/form-components/Input/InputLabeled.tsx
index 5a19af279..2992470ff 100644
--- a/app/react/components/form-components/Input/InputLabeled.tsx
+++ b/app/react/components/form-components/Input/InputLabeled.tsx
@@ -1,4 +1,5 @@
import { ComponentProps, InputHTMLAttributes } from 'react';
+import clsx from 'clsx';
import { InputGroup } from '../InputGroup';
@@ -6,21 +7,29 @@ export function InputLabeled({
label,
className,
size,
+ needsDeletion,
id,
+ required,
+ disabled,
...props
}: {
label: string;
className?: string;
size?: ComponentProps['size'];
+ needsDeletion?: boolean;
} & Omit, 'size' | 'children'>) {
return (
-
-
+
+
{label}
diff --git a/app/react/components/form-components/InputList/InputList.stories.tsx b/app/react/components/form-components/InputList/InputList.stories.tsx
index 8ef5a5e66..9689f27fa 100644
--- a/app/react/components/form-components/InputList/InputList.stories.tsx
+++ b/app/react/components/form-components/InputList/InputList.stories.tsx
@@ -12,7 +12,7 @@ const meta: Meta = {
export default meta;
-export { Defaults, ListWithInputAndSelect };
+export { Defaults, ListWithInputAndSelect, ListWithUndoDeletion };
function Defaults() {
const [values, setValues] = useState([{ value: '' }]);
@@ -26,6 +26,21 @@ function Defaults() {
);
}
+function ListWithUndoDeletion() {
+ const [values, setValues] = useState([
+ { value: 'Existing item', needsDeletion: false },
+ ]);
+
+ return (
+ setValues(value)}
+ canUndoDelete
+ />
+ );
+}
+
interface ListWithSelectItem {
value: number;
select: string;
diff --git a/app/react/components/form-components/InputList/InputList.tsx b/app/react/components/form-components/InputList/InputList.tsx
index 79ca4d082..61f588838 100644
--- a/app/react/components/form-components/InputList/InputList.tsx
+++ b/app/react/components/form-components/InputList/InputList.tsx
@@ -1,6 +1,7 @@
-import { ComponentType } from 'react';
+import { ComponentType, useRef } from 'react';
import { FormikErrors } from 'formik';
-import { ArrowDown, ArrowUp, Plus, Trash2 } from 'lucide-react';
+import { ArrowDown, ArrowUp, Plus, RotateCw, Trash2 } from 'lucide-react';
+import clsx from 'clsx';
import { Button } from '@@/buttons';
import { Tooltip } from '@@/Tip/Tooltip';
@@ -9,7 +10,7 @@ import { TextTip } from '@@/Tip/TextTip';
import { Input } from '../Input';
import { FormError } from '../FormError';
-import { arrayMove } from './utils';
+import { arrayMove, hasKey } from './utils';
type ArrElement = ArrType extends readonly (infer ElementType)[]
? ElementType
@@ -30,10 +31,12 @@ export interface ItemProps {
readOnly?: boolean;
// eslint-disable-next-line react/no-unused-prop-types
index: number;
+ needsDeletion?: boolean;
}
type Key = string | number;
type ChangeType = 'delete' | 'create' | 'update';
-export type DefaultType = { value: string };
+export type DefaultType = { value: string; needsDeletion?: boolean };
+type CanUndoDeleteItem = T & { needsDeletion: boolean };
type OnChangeEvent =
| {
@@ -64,6 +67,7 @@ interface Props {
addLabel?: string;
itemKeyGetter?(item: T, index: number): Key;
movable?: boolean;
+ canUndoDelete?: boolean;
errors?: ArrayError;
textTip?: string;
isAddButtonHidden?: boolean;
@@ -83,6 +87,7 @@ export function InputList({
addLabel = 'Add item',
itemKeyGetter = (item: T, index: number) => index,
movable,
+ canUndoDelete = false,
errors,
textTip,
isAddButtonHidden = false,
@@ -90,6 +95,7 @@ export function InputList({
readOnly,
'aria-label': ariaLabel,
}: Props) {
+ const initialItemsCount = useRef(value.length);
const isAddButtonVisible = !(isAddButtonHidden || readOnly);
return (
@@ -154,7 +160,7 @@ export function InputList({
/>
>
)}
- {!readOnly && (
+ {!readOnly && !canUndoDelete && (
({
icon={Trash2}
/>
)}
+ {!readOnly &&
+ canUndoDelete &&
+ hasKey(item, 'needsDeletion') && (
+
+ )}
);
@@ -224,6 +241,10 @@ export function InputList({
);
}
+ function handleToggleNeedsDeletion(key: Key, item: CanUndoDeleteItem) {
+ handleChangeItem(key, { ...item, needsDeletion: !item.needsDeletion });
+ }
+
function handleAdd() {
const newItem = itemBuilder();
onChange([...value, newItem], { type: 'create', item: newItem });
@@ -260,8 +281,8 @@ function DefaultItem({
onChange({ value: e.target.value })}
- className="!w-full"
- disabled={disabled}
+ className={clsx('!w-full', item.needsDeletion && 'striked')}
+ disabled={disabled || item.needsDeletion}
readOnly={readOnly}
/>
{error && {error} }
@@ -279,3 +300,51 @@ function renderDefaultItem(
);
}
+
+type CanUndoDeleteButtonProps = {
+ item: CanUndoDeleteItem;
+ itemIndex: number;
+ initialItemsCount: number;
+ handleRemoveItem(key: Key, item: T): void;
+ handleToggleNeedsDeletion(key: Key, item: T): void;
+};
+
+function CanUndoDeleteButton({
+ item,
+ itemIndex,
+ initialItemsCount,
+ handleRemoveItem,
+ handleToggleNeedsDeletion,
+}: CanUndoDeleteButtonProps) {
+ return (
+
+ {!item.needsDeletion && (
+
+ )}
+ {item.needsDeletion && (
+
+ )}
+
+ );
+
+ // if the item is new, we can just remove it, otherwise we need to toggle the needsDeletion flag
+ function handleDeleteClick() {
+ if (itemIndex < initialItemsCount) {
+ handleToggleNeedsDeletion(itemIndex, item);
+ } else {
+ handleRemoveItem(itemIndex, item);
+ }
+ }
+}
diff --git a/app/react/components/form-components/InputList/utils.ts b/app/react/components/form-components/InputList/utils.ts
index 41b7fc307..27a1a279d 100644
--- a/app/react/components/form-components/InputList/utils.ts
+++ b/app/react/components/form-components/InputList/utils.ts
@@ -35,3 +35,14 @@ export function arrayMove(array: Array, from: number, to: number) {
return index >= 0 && index <= array.length;
}
}
+
+export function hasKey(
+ value: unknown,
+ key: string | number | symbol
+): value is { needsDeletion: boolean } {
+ return isObject(value) && key in value;
+}
+
+function isObject(value: unknown): value is object {
+ return typeof value === 'object' && value !== null;
+}
diff --git a/app/react/kubernetes/applications/ApplicationForm/kubeEnvVarValidationSchema.ts b/app/react/kubernetes/applications/ApplicationForm/kubeEnvVarValidationSchema.ts
new file mode 100644
index 000000000..5800aac99
--- /dev/null
+++ b/app/react/kubernetes/applications/ApplicationForm/kubeEnvVarValidationSchema.ts
@@ -0,0 +1,26 @@
+import { SchemaOf, array, bool, object, string } from 'yup';
+
+import { EnvVar } from '@@/form-components/EnvironmentVariablesFieldset/types';
+import { buildUniquenessTest } from '@@/form-components/validate-unique';
+
+export function kubeEnvVarValidationSchema(): SchemaOf {
+ return array(
+ object({
+ name: string()
+ .required('Name is required')
+ .matches(
+ /^[a-zA-Z][a-zA-Z0-9_.-]*$/,
+ `This field must consist of alphabetic characters, digits, '_', '-', or '.', and must not start with a digit (e.g. 'my.env-name', or 'MY_ENV.NAME', or 'MyEnvName1'.`
+ ),
+ value: string().default(''),
+ needsDeletion: bool().default(false),
+ })
+ ).test(
+ 'unique',
+ 'This environment variable is already defined.',
+ buildUniquenessTest(
+ () => 'This environment variable is already defined.',
+ 'name'
+ )
+ );
+}