Fix Pipeline Parameters in Modals accept empty string defaults

openshift · Jun 2, 2021 · 24650c5 · 24650c5
1 parent 10b57ff
commit 24650c5
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 13 deletions.
diff --git a/...ages/pipelines-plugin/src/components/pipelines/modals/common/PipelineParameterSection.tsx b/...ages/pipelines-plugin/src/components/pipelines/modals/common/PipelineParameterSection.tsx
@@ -2,16 +2,18 @@ import * as React from 'react';
 import { FieldArray } from 'formik';
 import { useTranslation } from 'react-i18next';
 import { TextInputTypes } from '@patternfly/react-core';
-import { InputField } from '@console/shared';
-import { PipelineParam } from '../../../../utils/pipeline-augment';
+import { InputField, useFormikValidationFix } from '@console/shared';
 import FormSection from '@console/dev-console/src/components/import/section/FormSection';
+import { taskParamIsRequired } from '../../pipeline-builder/utils';
+import { ModalParameter } from './types';
 
 type ParametersSectionProps = {
-  parameters: PipelineParam[];
+  parameters: ModalParameter[];
 };
 
 const PipelineParameterSection: React.FC<ParametersSectionProps> = ({ parameters }) => {
   const { t } = useTranslation();
+  useFormikValidationFix(parameters);
   return (
     <FieldArray
       name="parameters"
@@ -22,12 +24,11 @@ const PipelineParameterSection: React.FC<ParametersSectionProps> = ({ parameters
             {parameters.map((parameter, index) => (
               <InputField
                 key={parameter.name}
-                name={`parameters.${index}.default`}
+                name={`parameters.${index}.value`}
                 type={TextInputTypes.text}
                 label={parameter.name}
                 helpText={parameter.description}
-                placeholder={t('pipelines-plugin~Name')}
-                required
+                required={taskParamIsRequired(parameter)}
               />
             ))}
           </FormSection>

diff --git a/.../packages/pipelines-plugin/src/components/pipelines/modals/common/__tests__/utils.spec.ts b/.../packages/pipelines-plugin/src/components/pipelines/modals/common/__tests__/utils.spec.ts
@@ -247,6 +247,7 @@ describe('PipelineAction testing getPipelineRunFromForm', () => {
           name: 'ParameterA',
           default: 'Default value',
           description: 'Description',
+          value: 'Updated value',
         },
       ],
       resources: [],
@@ -267,7 +268,7 @@ describe('PipelineAction testing getPipelineRunFromForm', () => {
         params: [
           {
             name: 'ParameterA',
-            value: 'Default value',
+            value: 'Updated value',
           },
         ],
         resources: [],

diff --git a/frontend/packages/pipelines-plugin/src/components/pipelines/modals/common/types.ts b/frontend/packages/pipelines-plugin/src/components/pipelines/modals/common/types.ts
@@ -38,9 +38,13 @@ export type PipelineModalFormWorkspace = {
       };
 };
 
+export type ModalParameter = PipelineParam & {
+  value?: string | string[];
+};
+
 export type CommonPipelineModalFormikValues = FormikValues & {
   namespace: string;
-  parameters: PipelineParam[];
+  parameters: ModalParameter[];
   resources: PipelineModalFormResource[];
   workspaces: PipelineModalFormWorkspace[];
 };
diff --git a/frontend/packages/pipelines-plugin/src/components/pipelines/modals/common/utils.ts b/frontend/packages/pipelines-plugin/src/components/pipelines/modals/common/utils.ts
@@ -7,6 +7,7 @@ import {
   PipelineRun,
   PipelineRunInlineResource,
   PipelineRunInlineResourceParam,
+  PipelineRunParam,
   PipelineRunReferenceResource,
   PipelineRunResource,
   PipelineWorkspace,
@@ -163,7 +164,10 @@ export const convertPipelineToModalData = (
 
   return {
     namespace,
-    parameters: params || [],
+    parameters: (params || []).map((param) => ({
+      ...param,
+      value: param.default, // setup the default if it exists
+    })),
     resources: (resources || []).map((resource: PipelineResource) => ({
       name: resource.name,
       selection: alwaysCreateResources ? CREATE_PIPELINE_RESOURCE : null,
@@ -226,7 +230,7 @@ export const getPipelineRunFromForm = (
       pipelineRef: {
         name: pipeline.metadata.name,
       },
-      params: getPipelineRunParams(parameters),
+      params: parameters.map(({ name, value }): PipelineRunParam => ({ name, value })),
       resources: resources.map(convertResources),
       workspaces: getPipelineRunWorkspaces(workspaces),
     },

diff --git a/...tend/packages/pipelines-plugin/src/components/pipelines/modals/common/validation-utils.ts b/...tend/packages/pipelines-plugin/src/components/pipelines/modals/common/validation-utils.ts
@@ -1,6 +1,7 @@
 import { TFunction } from 'i18next';
 import * as yup from 'yup';
 import { PipelineResourceType, VolumeTypes } from '../../const';
+import { taskParamIsRequired } from '../../pipeline-builder/utils';
 import { CREATE_PIPELINE_RESOURCE } from './const';
 
 export const validateResourceType = (t: TFunction) =>
@@ -105,8 +106,15 @@ const commonPipelineSchema = (t: TFunction) =>
     parameters: yup.array().of(
       yup.object().shape({
         name: yup.string().required(t('pipelines-plugin~Required')),
+        default: yup.string(),
         description: yup.string(),
-        default: yup.string().required(t('pipelines-plugin~Required')),
+        value: yup
+          .string()
+          .test('test-if-param-can-be-empty', t('pipelines-plugin~Required'), function(
+            value: string,
+          ) {
+            return taskParamIsRequired(this.parent) ? !!value : true;
+          }),
       }),
     ),
     resources: formResources(t),

diff --git a/frontend/packages/pipelines-plugin/src/components/pipelines/pipeline-builder/utils.ts b/frontend/packages/pipelines-plugin/src/components/pipelines/pipeline-builder/utils.ts
@@ -5,8 +5,8 @@ import { EditorType } from '@console/shared/src/components/synced-editor/editor-
 import { PipelineModel, TaskModel } from '../../../models';
 import {
   Pipeline,
+  PipelineParam,
   PipelineResourceTask,
-  PipelineResourceTaskParam,
   PipelineTask,
 } from '../../../utils/pipeline-augment';
 import { removeEmptyDefaultFromPipelineParams } from '../detail-page-tabs/utils';
@@ -28,7 +28,7 @@ export const getErrorMessage = (errorTypes: TaskErrorType[], errorMap: TaskError
   return hasRequestedError.length > 0 ? TASK_ERROR_STRINGS[hasRequestedError[0]] : null;
 };
 
-export const taskParamIsRequired = (param: PipelineResourceTaskParam): boolean => {
+export const taskParamIsRequired = (param: PipelineParam): boolean => {
   return !('default' in param);
 };