From d6aa12fd20758820e8188dd20753893fb2f3a08f Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Wed, 8 Jan 2020 15:00:11 -0800 Subject: [PATCH 01/10] Basic plumbing for using default values and ensuring booleans always have a value --- src/common/utils.ts | 6 ++--- .../inputHelpers/boolean.ts | 14 +++++++++- .../inputHelpers/collection.ts | 10 ++++++- .../inputHelpers/constants.ts | 11 ++++++++ .../inputHelpers/datetime.ts | 7 +++++ .../inputHelpers/duration.ts | 8 ++++++ .../LaunchWorkflowForm/inputHelpers/float.ts | 7 +++++ .../inputHelpers/inputHelpers.ts | 27 ++++++++++++++++++- .../inputHelpers/integer.ts | 7 +++++ .../LaunchWorkflowForm/inputHelpers/none.ts | 1 + .../LaunchWorkflowForm/inputHelpers/string.ts | 8 ++++++ .../inputHelpers/test/inputHelpers.test.ts | 7 +++++ .../LaunchWorkflowForm/inputHelpers/types.ts | 10 +++++-- .../LaunchWorkflowForm/inputHelpers/utils.ts | 10 +++++++ .../test/LaunchWorkflowForm.test.tsx | 5 +--- .../Launch/LaunchWorkflowForm/types.ts | 11 +++++--- .../LaunchWorkflowForm/useFormInputsState.ts | 5 ++-- .../useLaunchWorkflowFormState.ts | 12 +++++++-- src/models/Common/types.ts | 4 +++ 19 files changed, 150 insertions(+), 20 deletions(-) create mode 100644 src/components/Launch/LaunchWorkflowForm/inputHelpers/utils.ts diff --git a/src/common/utils.ts b/src/common/utils.ts index e727f243b..cdf07cb5b 100644 --- a/src/common/utils.ts +++ b/src/common/utils.ts @@ -87,9 +87,9 @@ export function createCorsProxyURL(path: string) { } /** Returns entires for an object, sorted lexicographically */ -export function sortedObjectEntries( - object: Object -): ReturnType { +export function sortedObjectEntries(object: { + [s: string]: T; +}): [string, T][] { return Object.entries(object).sort((a, b) => a[0].localeCompare(b[0])); } diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/boolean.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/boolean.ts index 380accf95..4c25ca705 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/boolean.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/boolean.ts @@ -1,6 +1,9 @@ import { Core } from 'flyteidl'; +import { Literal } from 'models'; import { InputValue } from '../types'; +import { literalValuePaths } from './constants'; import { ConverterInput, InputHelper } from './types'; +import { extractLiteralWithCheck } from './utils'; /** Checks that a value is an acceptable boolean value. These can be * an actual Boolean instance, any variant of ('T', 'F', 'TRUE', 'FALSE') or @@ -38,6 +41,13 @@ function toLiteral({ value }: ConverterInput): Core.ILiteral { return { scalar: { primitive: { boolean: parseBoolean(value) } } }; } +function fromLiteral(literal: Literal): InputValue { + return extractLiteralWithCheck( + literal, + literalValuePaths.scalarBoolean + ); +} + function validate({ value }: ConverterInput) { if (!isValidBoolean(value)) { throw new Error('Value is not a valid boolean'); @@ -45,6 +55,8 @@ function validate({ value }: ConverterInput) { } export const booleanHelper: InputHelper = { + fromLiteral, toLiteral, - validate + validate, + defaultValue: false }; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/collection.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/collection.ts index 6cc171911..dfebf8188 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/collection.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/collection.ts @@ -1,4 +1,6 @@ import { Core } from 'flyteidl'; +import { Literal } from 'models'; +import { InputValue } from '../types'; import { literalNone } from './constants'; import { getHelperForInput } from './getHelperForInput'; import { parseJSON } from './parseJson'; @@ -12,7 +14,12 @@ function parseCollection(list: string) { return parsed; } -export function toLiteral({ +function fromLiteral(literal: Literal): InputValue { + // TODO + return ''; +} + +function toLiteral({ value, typeDefinition: { subtype } }: ConverterInput): Core.ILiteral { @@ -63,6 +70,7 @@ function validate({ value }: ConverterInput) { } export const collectionHelper: InputHelper = { + fromLiteral, toLiteral, validate }; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/constants.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/constants.ts index b7e8c5b8e..03324b1bd 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/constants.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/constants.ts @@ -6,3 +6,14 @@ export function literalNone(): Core.ILiteral { } export const allowedDateFormats = [ISO_8601, RFC_2822]; + +const primitivePath = 'scalar.primitive'; + +export const literalValuePaths = { + scalarBoolean: `${primitivePath}.boolean`, + scalarDatetime: `${primitivePath}.datetime`, + scalarDuration: `${primitivePath}.duration`, + scalarFloat: `${primitivePath}.floatValue`, + scalarInteger: `${primitivePath}.integer`, + scalarString: `${primitivePath}.stringValue` +}; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/datetime.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/datetime.ts index 4c9b3cc68..569e39230 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/datetime.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/datetime.ts @@ -1,5 +1,6 @@ import { dateToTimestamp } from 'common/utils'; import { Core } from 'flyteidl'; +import { Literal } from 'models'; import { utc as moment } from 'moment'; import { InputValue } from '../types'; import { allowedDateFormats } from './constants'; @@ -11,6 +12,11 @@ function parseDate(value: InputValue) { : moment(value.toString(), allowedDateFormats).toDate(); } +function fromLiteral(literal: Literal): InputValue { + // TODO + return ''; +} + function toLiteral({ value }: ConverterInput): Core.ILiteral { const datetime = dateToTimestamp(parseDate(value)); return { @@ -26,6 +32,7 @@ function validate({ value }: ConverterInput) { } export const datetimeHelper: InputHelper = { + fromLiteral, toLiteral, validate }; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/duration.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/duration.ts index 24fa4af73..6c09800ef 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/duration.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/duration.ts @@ -1,8 +1,15 @@ import { millisecondsToDuration } from 'common/utils'; import { Core } from 'flyteidl'; +import { Literal } from 'models'; +import { InputValue } from '../types'; import { isValidFloat } from './float'; import { ConverterInput, InputHelper } from './types'; +function fromLiteral(literal: Literal): InputValue { + // TODO + return ''; +} + function toLiteral({ value }: ConverterInput): Core.ILiteral { const parsed = typeof value === 'number' ? value : parseInt(value.toString(), 10); @@ -19,6 +26,7 @@ function validate({ value }: ConverterInput) { } export const durationHelper: InputHelper = { + fromLiteral, toLiteral, validate }; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/float.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/float.ts index cd3d09726..5e4e95e43 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/float.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/float.ts @@ -1,7 +1,13 @@ import { Core } from 'flyteidl'; +import { Literal } from 'models'; import { InputValue } from '../types'; import { ConverterInput, InputHelper } from './types'; +function fromLiteral(literal: Literal): InputValue { + // TODO + return ''; +} + function toLiteral({ value }: ConverterInput): Core.ILiteral { const floatValue = typeof value === 'number' ? value : parseFloat(value.toString()); @@ -27,6 +33,7 @@ function validate({ value }: ConverterInput) { } export const floatHelper: InputHelper = { + fromLiteral, toLiteral, validate }; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts index 82f1c4232..dc70ac04c 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts @@ -1,6 +1,7 @@ import { ValueError } from 'errors'; import { Core } from 'flyteidl'; -import { InputProps } from '../types'; +import { Literal } from 'models'; +import { InputProps, InputTypeDefinition, InputValue } from '../types'; import { literalNone } from './constants'; import { getHelperForInput } from './getHelperForInput'; @@ -15,6 +16,30 @@ export function inputToLiteral(input: ToLiteralParams): Core.ILiteral { return toLiteral({ value, typeDefinition }); } +export function defaultValueForInputType( + typeDefinition: InputTypeDefinition +): InputValue | undefined { + return getHelperForInput(typeDefinition.type).defaultValue; +} + +export function literalToInputValue( + typeDefinition: InputTypeDefinition, + literal: Literal +): InputValue | undefined { + const { defaultValue, fromLiteral } = getHelperForInput( + typeDefinition.type + ); + + try { + return fromLiteral(literal); + } catch (e) { + // If something goes wrong (most likely malformed default value input), + // we'll return the system default value. + console.error((e as Error).message); + return defaultValue; + } +} + type ValidationParams = Pick; export function validateInput(input: ValidationParams) { if (input.value == null) { diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/integer.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/integer.ts index 624203294..3b074ddae 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/integer.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/integer.ts @@ -1,10 +1,16 @@ import { Core } from 'flyteidl'; import * as Long from 'long'; +import { Literal } from 'models'; import { InputValue } from '../types'; import { ConverterInput, InputHelper } from './types'; const integerRegexPattern = /^-?[0-9]+$/; +function fromLiteral(literal: Literal): InputValue { + // TODO + return ''; +} + function toLiteral({ value }: ConverterInput): Core.ILiteral { const integer = value instanceof Long ? value : Long.fromString(value.toString()); @@ -33,6 +39,7 @@ function validate({ value }: ConverterInput) { } export const integerHelper: InputHelper = { + fromLiteral, toLiteral, validate }; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/none.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/none.ts index 4c9230ef1..3205ccf41 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/none.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/none.ts @@ -2,6 +2,7 @@ import { literalNone } from './constants'; import { InputHelper } from './types'; export const noneHelper: InputHelper = { + fromLiteral: () => '', toLiteral: literalNone, validate: () => {} }; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/string.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/string.ts index d98c5acf3..8de47d836 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/string.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/string.ts @@ -1,6 +1,13 @@ import { Core } from 'flyteidl'; +import { Literal } from 'models'; +import { InputValue } from '../types'; import { ConverterInput, InputHelper } from './types'; +function fromLiteral(literal: Literal): InputValue { + // TODO + return ''; +} + function toLiteral({ value }: ConverterInput): Core.ILiteral { const stringValue = typeof value === 'string' ? value : value.toString(); return { scalar: { primitive: { stringValue } } }; @@ -13,6 +20,7 @@ function validate({ value }: ConverterInput) { } export const stringHelper: InputHelper = { + fromLiteral, toLiteral, validate }; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts index eb65630e7..7a0d3754a 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts @@ -34,6 +34,13 @@ function makeNestedCollectionInput(type: InputType, value: string): InputProps { }; } +describe('literalToInputValue', () => { + // TODO: test cases for all of the input types + + // TODO + it('should return system default if parsing literal fails', () => {}); +}); + describe('inputToLiteral', () => { describe('Primitives', () => { literalTestCases.map(([type, input, output]) => diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/types.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/types.ts index e7b28f517..81f426cd5 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/types.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/types.ts @@ -1,4 +1,5 @@ import { Core } from 'flyteidl'; +import { Literal } from 'models'; import { InputTypeDefinition, InputValue } from '../types'; export interface ConverterInput { @@ -6,9 +7,14 @@ export interface ConverterInput { typeDefinition: InputTypeDefinition; } -export type LiteralConverterFn = (input: ConverterInput) => Core.ILiteral; +export type InputToLiteralConverterFn = ( + input: ConverterInput +) => Core.ILiteral; +export type LiteralToInputConterterFn = (literal: Literal) => InputValue; export interface InputHelper { - toLiteral: LiteralConverterFn; + defaultValue?: InputValue; + toLiteral: InputToLiteralConverterFn; + fromLiteral: LiteralToInputConterterFn; /** Will throw in the case of a failed validation */ validate: (input: ConverterInput) => void; } diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/utils.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/utils.ts new file mode 100644 index 000000000..b32b5660f --- /dev/null +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/utils.ts @@ -0,0 +1,10 @@ +import { get } from 'lodash'; +import { Literal } from 'models'; + +export function extractLiteralWithCheck(literal: Literal, path: string): T { + const value = get(literal, path); + if (value === undefined) { + throw new Error(`Failed to extract literal value with path ${path}`); + } + return value as T; +} diff --git a/src/components/Launch/LaunchWorkflowForm/test/LaunchWorkflowForm.test.tsx b/src/components/Launch/LaunchWorkflowForm/test/LaunchWorkflowForm.test.tsx index 9f07640cd..6ac0254a9 100644 --- a/src/components/Launch/LaunchWorkflowForm/test/LaunchWorkflowForm.test.tsx +++ b/src/components/Launch/LaunchWorkflowForm/test/LaunchWorkflowForm.test.tsx @@ -375,10 +375,7 @@ describe('LaunchWorkflowForm', () => { }); describe('Input Values', () => { - /* TODO: Un-skip this when https://github.com/lyft/flyte/issues/18 - * is fixed. - */ - it.skip('Should send false for untouched toggles', async () => { + it('Should send false for untouched toggles', async () => { let inputs: Core.ILiteralMap = {}; mockCreateWorkflowExecution.mockImplementation( ({ diff --git a/src/components/Launch/LaunchWorkflowForm/types.ts b/src/components/Launch/LaunchWorkflowForm/types.ts index 046d76604..53a9d93b5 100644 --- a/src/components/Launch/LaunchWorkflowForm/types.ts +++ b/src/components/Launch/LaunchWorkflowForm/types.ts @@ -77,7 +77,10 @@ export interface InputProps { onChange: InputChangeHandler; } -export type ParsedInput = Pick< - InputProps, - 'description' | 'label' | 'name' | 'required' | 'typeDefinition' ->; +export interface ParsedInput + extends Pick< + InputProps, + 'description' | 'label' | 'name' | 'required' | 'typeDefinition' + > { + defaultValue?: InputValue; +} diff --git a/src/components/Launch/LaunchWorkflowForm/useFormInputsState.ts b/src/components/Launch/LaunchWorkflowForm/useFormInputsState.ts index 87ca216ba..a048b756f 100644 --- a/src/components/Launch/LaunchWorkflowForm/useFormInputsState.ts +++ b/src/components/Launch/LaunchWorkflowForm/useFormInputsState.ts @@ -1,5 +1,4 @@ import { useDebouncedValue } from 'components/hooks/useDebouncedValue'; -import { ValidationError, ValueError } from 'errors'; import { Core } from 'flyteidl'; import { useEffect, useState } from 'react'; import { validateInput } from './inputHelpers/inputHelpers'; @@ -19,7 +18,9 @@ interface FormInputsState { } function useFormInputState(parsedInput: ParsedInput): FormInputState { - const [value, setValue] = useState(); + const [value, setValue] = useState( + parsedInput.defaultValue + ); const [error, setError] = useState(); const validationValue = useDebouncedValue(value, debounceDelay); diff --git a/src/components/Launch/LaunchWorkflowForm/useLaunchWorkflowFormState.ts b/src/components/Launch/LaunchWorkflowForm/useLaunchWorkflowFormState.ts index 7067a4eac..38431a96d 100644 --- a/src/components/Launch/LaunchWorkflowForm/useLaunchWorkflowFormState.ts +++ b/src/components/Launch/LaunchWorkflowForm/useLaunchWorkflowFormState.ts @@ -18,6 +18,10 @@ import { } from 'models'; import { useEffect, useMemo, useRef, useState } from 'react'; import { history, Routes } from 'routes'; +import { + defaultValueForInputType, + literalToInputValue +} from './inputHelpers/inputHelpers'; import { SearchableSelectorOption } from './SearchableSelector'; import { LaunchWorkflowFormInputsRef, @@ -59,9 +63,13 @@ function getInputs(workflow: Workflow, launchPlan: LaunchPlan): ParsedInput[] { ); const label = formatLabelWithType(name, typeDefinition); - // TODO: - // Extract default value for more specific type (maybe just for simple) + const defaultValue = + parameter.default !== undefined + ? literalToInputValue(typeDefinition, parameter.default) + : defaultValueForInputType(typeDefinition); + return { + defaultValue, description, label, name, diff --git a/src/models/Common/types.ts b/src/models/Common/types.ts index 534b1d21e..895c130a8 100644 --- a/src/models/Common/types.ts +++ b/src/models/Common/types.ts @@ -1,4 +1,5 @@ import { Admin, Core, Protobuf } from 'flyteidl'; +import { Collection } from 'react-virtualized'; /* --- BEGIN flyteidl type aliases --- */ /** These are types shared across multiple sections of the data model. Most of @@ -59,6 +60,8 @@ export interface Error extends RequiredNonNullable {} export interface Literal extends Core.Literal { value: keyof Core.ILiteral; + collection?: Core.ILiteralCollection; + map?: Core.ILiteralMap; scalar?: Scalar; } @@ -72,6 +75,7 @@ export interface LiteralMapBlob extends Admin.ILiteralMapBlob { } export interface Scalar extends Core.IScalar { + primitive?: Primitive; value: keyof Core.IScalar; } From 3d525a8d1becf1fc8133916c9f32fb1948ff9d98 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Wed, 8 Jan 2020 15:36:43 -0800 Subject: [PATCH 02/10] Implementing remaining literal parsers --- .../inputHelpers/boolean.ts | 3 +- .../inputHelpers/collection.ts | 3 +- .../inputHelpers/datetime.ts | 17 +++-- .../inputHelpers/duration.ts | 16 +++-- .../LaunchWorkflowForm/inputHelpers/float.ts | 11 ++-- .../inputHelpers/inputHelpers.ts | 2 +- .../inputHelpers/integer.ts | 11 ++-- .../LaunchWorkflowForm/inputHelpers/none.ts | 2 +- .../LaunchWorkflowForm/inputHelpers/string.ts | 11 ++-- .../inputHelpers/test/inputHelpers.test.ts | 23 ++++++- .../inputHelpers/test/testCases.ts | 64 ++++++++++++++++++- .../LaunchWorkflowForm/inputHelpers/types.ts | 5 +- .../LaunchWorkflowForm/inputHelpers/utils.ts | 7 +- .../test/LaunchWorkflowForm.test.tsx | 2 + 14 files changed, 138 insertions(+), 39 deletions(-) diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/boolean.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/boolean.ts index 4c25ca705..668f47158 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/boolean.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/boolean.ts @@ -1,5 +1,4 @@ import { Core } from 'flyteidl'; -import { Literal } from 'models'; import { InputValue } from '../types'; import { literalValuePaths } from './constants'; import { ConverterInput, InputHelper } from './types'; @@ -41,7 +40,7 @@ function toLiteral({ value }: ConverterInput): Core.ILiteral { return { scalar: { primitive: { boolean: parseBoolean(value) } } }; } -function fromLiteral(literal: Literal): InputValue { +function fromLiteral(literal: Core.ILiteral): InputValue { return extractLiteralWithCheck( literal, literalValuePaths.scalarBoolean diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/collection.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/collection.ts index dfebf8188..beab04b32 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/collection.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/collection.ts @@ -1,5 +1,4 @@ import { Core } from 'flyteidl'; -import { Literal } from 'models'; import { InputValue } from '../types'; import { literalNone } from './constants'; import { getHelperForInput } from './getHelperForInput'; @@ -14,7 +13,7 @@ function parseCollection(list: string) { return parsed; } -function fromLiteral(literal: Literal): InputValue { +function fromLiteral(literal: Core.ILiteral): InputValue { // TODO return ''; } diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/datetime.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/datetime.ts index 569e39230..f564cd30d 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/datetime.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/datetime.ts @@ -1,10 +1,10 @@ -import { dateToTimestamp } from 'common/utils'; -import { Core } from 'flyteidl'; -import { Literal } from 'models'; +import { dateToTimestamp, timestampToDate } from 'common/utils'; +import { Core, Protobuf } from 'flyteidl'; import { utc as moment } from 'moment'; import { InputValue } from '../types'; -import { allowedDateFormats } from './constants'; +import { allowedDateFormats, literalValuePaths } from './constants'; import { ConverterInput, InputHelper } from './types'; +import { extractLiteralWithCheck } from './utils'; function parseDate(value: InputValue) { return value instanceof Date @@ -12,9 +12,12 @@ function parseDate(value: InputValue) { : moment(value.toString(), allowedDateFormats).toDate(); } -function fromLiteral(literal: Literal): InputValue { - // TODO - return ''; +function fromLiteral(literal: Core.ILiteral): InputValue { + const value = extractLiteralWithCheck( + literal, + literalValuePaths.scalarDatetime + ); + return timestampToDate(value); } function toLiteral({ value }: ConverterInput): Core.ILiteral { diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/duration.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/duration.ts index 6c09800ef..065ddd80f 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/duration.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/duration.ts @@ -1,13 +1,17 @@ -import { millisecondsToDuration } from 'common/utils'; -import { Core } from 'flyteidl'; -import { Literal } from 'models'; +import { durationToMilliseconds, millisecondsToDuration } from 'common/utils'; +import { Core, Protobuf } from 'flyteidl'; import { InputValue } from '../types'; +import { literalValuePaths } from './constants'; import { isValidFloat } from './float'; import { ConverterInput, InputHelper } from './types'; +import { extractLiteralWithCheck } from './utils'; -function fromLiteral(literal: Literal): InputValue { - // TODO - return ''; +function fromLiteral(literal: Core.ILiteral): InputValue { + const value = extractLiteralWithCheck( + literal, + literalValuePaths.scalarDuration + ); + return durationToMilliseconds(value); } function toLiteral({ value }: ConverterInput): Core.ILiteral { diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/float.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/float.ts index 5e4e95e43..9dbaf2890 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/float.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/float.ts @@ -1,11 +1,14 @@ import { Core } from 'flyteidl'; -import { Literal } from 'models'; import { InputValue } from '../types'; +import { literalValuePaths } from './constants'; import { ConverterInput, InputHelper } from './types'; +import { extractLiteralWithCheck } from './utils'; -function fromLiteral(literal: Literal): InputValue { - // TODO - return ''; +function fromLiteral(literal: Core.ILiteral): InputValue { + return extractLiteralWithCheck( + literal, + literalValuePaths.scalarFloat + ); } function toLiteral({ value }: ConverterInput): Core.ILiteral { diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts index dc70ac04c..8a7873f8e 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts @@ -24,7 +24,7 @@ export function defaultValueForInputType( export function literalToInputValue( typeDefinition: InputTypeDefinition, - literal: Literal + literal: Core.ILiteral ): InputValue | undefined { const { defaultValue, fromLiteral } = getHelperForInput( typeDefinition.type diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/integer.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/integer.ts index 3b074ddae..d0f37d804 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/integer.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/integer.ts @@ -1,14 +1,17 @@ import { Core } from 'flyteidl'; import * as Long from 'long'; -import { Literal } from 'models'; import { InputValue } from '../types'; +import { literalValuePaths } from './constants'; import { ConverterInput, InputHelper } from './types'; +import { extractLiteralWithCheck } from './utils'; const integerRegexPattern = /^-?[0-9]+$/; -function fromLiteral(literal: Literal): InputValue { - // TODO - return ''; +function fromLiteral(literal: Core.ILiteral): InputValue { + return extractLiteralWithCheck( + literal, + literalValuePaths.scalarInteger + ).toString(); } function toLiteral({ value }: ConverterInput): Core.ILiteral { diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/none.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/none.ts index 3205ccf41..273679c21 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/none.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/none.ts @@ -2,7 +2,7 @@ import { literalNone } from './constants'; import { InputHelper } from './types'; export const noneHelper: InputHelper = { - fromLiteral: () => '', + fromLiteral: () => undefined, toLiteral: literalNone, validate: () => {} }; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/string.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/string.ts index 8de47d836..a56cc9bac 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/string.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/string.ts @@ -1,11 +1,14 @@ import { Core } from 'flyteidl'; -import { Literal } from 'models'; import { InputValue } from '../types'; +import { literalValuePaths } from './constants'; import { ConverterInput, InputHelper } from './types'; +import { extractLiteralWithCheck } from './utils'; -function fromLiteral(literal: Literal): InputValue { - // TODO - return ''; +function fromLiteral(literal: Core.ILiteral): InputValue { + return extractLiteralWithCheck( + literal, + literalValuePaths.scalarString + ); } function toLiteral({ value }: ConverterInput): Core.ILiteral { diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts index 7a0d3754a..5474287f6 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts @@ -1,6 +1,14 @@ import { InputProps, InputType } from '../../types'; -import { inputToLiteral, validateInput } from '../inputHelpers'; -import { literalTestCases, validityTestCases } from './testCases'; +import { + inputToLiteral, + literalToInputValue, + validateInput +} from '../inputHelpers'; +import { + literalTestCases, + literalToInputTestCases, + validityTestCases +} from './testCases'; const baseInputProps: InputProps = { description: 'test', @@ -35,7 +43,16 @@ function makeNestedCollectionInput(type: InputType, value: string): InputProps { } describe('literalToInputValue', () => { - // TODO: test cases for all of the input types + describe.only('Primitives', () => { + literalToInputTestCases.map(([type, input, output]) => + it(`Should correctly convert ${type}: ${JSON.stringify( + input.scalar!.primitive + )}`, () => { + const result = literalToInputValue({ type }, input); + expect(result).toEqual(output); + }) + ); + }); // TODO it('should return system default if parsing literal fails', () => {}); diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/testCases.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/testCases.ts index 081b40bff..c5ee8bc42 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/testCases.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/testCases.ts @@ -1,7 +1,8 @@ import { dateToTimestamp, millisecondsToDuration } from 'common/utils'; import { Core } from 'flyteidl'; import * as Long from 'long'; -import { InputType } from '../../types'; +import { InputType, InputValue } from '../../types'; +import { literalNone } from '../constants'; // Defines type of value, input, and expected value of innermost `IScalar` type PrimitiveTestParams = [InputType, any, Core.IPrimitive]; @@ -97,3 +98,64 @@ export const literalTestCases: PrimitiveTestParams[] = [ [InputType.String, '', { stringValue: '' }], [InputType.String, 'abcdefg', { stringValue: 'abcdefg' }] ]; + +function primitiveLiteral(primitive: Core.IPrimitive): Core.ILiteral { + return { scalar: { primitive } }; +} + +type InputToLiteralTestParams = [ + InputType, + Core.ILiteral, + InputValue | undefined +]; +export const literalToInputTestCases: InputToLiteralTestParams[] = [ + [InputType.Boolean, primitiveLiteral({ boolean: true }), true], + [InputType.Boolean, primitiveLiteral({ boolean: false }), false], + [ + InputType.Datetime, + primitiveLiteral({ + datetime: dateToTimestamp(new Date(validDateString)) + }), + new Date(validDateString) + ], + [ + InputType.Duration, + primitiveLiteral({ duration: millisecondsToDuration(0) }), + 0 + ], + [ + InputType.Duration, + primitiveLiteral({ duration: millisecondsToDuration(10000) }), + 10000 + ], + [ + InputType.Duration, + primitiveLiteral({ duration: millisecondsToDuration(1.5) }), + 1.5 + ], + [InputType.Float, primitiveLiteral({ floatValue: 0 }), 0], + [InputType.Float, primitiveLiteral({ floatValue: -1.5 }), -1.5], + [InputType.Float, primitiveLiteral({ floatValue: 1.5 }), 1.5], + [InputType.Float, primitiveLiteral({ floatValue: 1.25e10 }), 1.25e10], + // Integers will be returned as strings because they may overflow numbers + [InputType.Integer, primitiveLiteral({ integer: Long.fromNumber(0) }), '0'], + [InputType.Integer, primitiveLiteral({ integer: Long.fromNumber(1) }), '1'], + [ + InputType.Integer, + primitiveLiteral({ integer: Long.fromNumber(-1) }), + '-1' + ], + [ + InputType.Integer, + primitiveLiteral({ integer: Long.MAX_VALUE }), + Long.MAX_VALUE.toString() + ], + [ + InputType.Integer, + primitiveLiteral({ integer: Long.MIN_VALUE }), + Long.MIN_VALUE.toString() + ], + [InputType.String, primitiveLiteral({ stringValue: '' }), ''], + [InputType.String, primitiveLiteral({ stringValue: 'abcdefg' }), 'abcdefg'], + [InputType.None, literalNone(), undefined] +]; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/types.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/types.ts index 81f426cd5..514896035 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/types.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/types.ts @@ -1,5 +1,4 @@ import { Core } from 'flyteidl'; -import { Literal } from 'models'; import { InputTypeDefinition, InputValue } from '../types'; export interface ConverterInput { @@ -10,7 +9,9 @@ export interface ConverterInput { export type InputToLiteralConverterFn = ( input: ConverterInput ) => Core.ILiteral; -export type LiteralToInputConterterFn = (literal: Literal) => InputValue; +export type LiteralToInputConterterFn = ( + literal: Core.ILiteral +) => InputValue | undefined; export interface InputHelper { defaultValue?: InputValue; toLiteral: InputToLiteralConverterFn; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/utils.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/utils.ts index b32b5660f..075bf139e 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/utils.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/utils.ts @@ -1,7 +1,10 @@ +import { Core } from 'flyteidl'; import { get } from 'lodash'; -import { Literal } from 'models'; -export function extractLiteralWithCheck(literal: Literal, path: string): T { +export function extractLiteralWithCheck( + literal: Core.ILiteral, + path: string +): T { const value = get(literal, path); if (value === undefined) { throw new Error(`Failed to extract literal value with path ${path}`); diff --git a/src/components/Launch/LaunchWorkflowForm/test/LaunchWorkflowForm.test.tsx b/src/components/Launch/LaunchWorkflowForm/test/LaunchWorkflowForm.test.tsx index 6ac0254a9..6c6a6889f 100644 --- a/src/components/Launch/LaunchWorkflowForm/test/LaunchWorkflowForm.test.tsx +++ b/src/components/Launch/LaunchWorkflowForm/test/LaunchWorkflowForm.test.tsx @@ -400,6 +400,8 @@ describe('LaunchWorkflowForm', () => { ); expect(value).toBe(false); }); + + it('should use default values when provided', async () => {}); }); }); }); From 814210f84915c002cd2b66c96a930b7510aa2005 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Wed, 8 Jan 2020 15:45:54 -0800 Subject: [PATCH 03/10] Adding tests for using default values --- .../inputHelpers/test/inputHelpers.test.ts | 2 +- .../test/LaunchWorkflowForm.test.tsx | 26 ++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts index 5474287f6..fdb0e762d 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts @@ -43,7 +43,7 @@ function makeNestedCollectionInput(type: InputType, value: string): InputProps { } describe('literalToInputValue', () => { - describe.only('Primitives', () => { + describe('Primitives', () => { literalToInputTestCases.map(([type, input, output]) => it(`Should correctly convert ${type}: ${JSON.stringify( input.scalar!.primitive diff --git a/src/components/Launch/LaunchWorkflowForm/test/LaunchWorkflowForm.test.tsx b/src/components/Launch/LaunchWorkflowForm/test/LaunchWorkflowForm.test.tsx index 6c6a6889f..1a4f9613a 100644 --- a/src/components/Launch/LaunchWorkflowForm/test/LaunchWorkflowForm.test.tsx +++ b/src/components/Launch/LaunchWorkflowForm/test/LaunchWorkflowForm.test.tsx @@ -14,6 +14,7 @@ import { APIContext } from 'components/data/apiContext'; import { muiTheme } from 'components/Theme'; import { Core } from 'flyteidl'; import { get, mapValues } from 'lodash'; +import * as Long from 'long'; import { createWorkflowExecution, CreateWorkflowExecutionArguments, @@ -23,6 +24,7 @@ import { LaunchPlan, listLaunchPlans, listWorkflows, + Literal, NamedEntityIdentifier, Variable, Workflow @@ -401,7 +403,29 @@ describe('LaunchWorkflowForm', () => { expect(value).toBe(false); }); - it('should use default values when provided', async () => {}); + it('should use default values when provided', async () => { + // Add defaults for the string/integer inputs and check that they are + // correctly populated + const parameters = mockLaunchPlans[0].closure!.expectedInputs + .parameters; + parameters[stringInputName].default = { + scalar: { primitive: { stringValue: 'abc' } } + } as Literal; + parameters[integerInputName].default = { + scalar: { primitive: { integer: Long.fromNumber(10000) } } + } as Literal; + mockGetLaunchPlan.mockResolvedValue(mockLaunchPlans[0]); + + const { getByLabelText } = renderForm(); + await wait(); + + expect( + getByLabelText(stringInputName, { exact: false }) + ).toHaveValue('abc'); + expect( + getByLabelText(integerInputName, { exact: false }) + ).toHaveValue('10000'); + }); }); }); }); From b789fd1a78989b3c06228e089c53d9bd396b2f3e Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Wed, 8 Jan 2020 15:50:02 -0800 Subject: [PATCH 04/10] Filling in remaining test --- .../LaunchWorkflowForm/inputHelpers/inputHelpers.ts | 2 +- .../inputHelpers/test/inputHelpers.test.ts | 13 +++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts index 8a7873f8e..11ebc4631 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts @@ -35,7 +35,7 @@ export function literalToInputValue( } catch (e) { // If something goes wrong (most likely malformed default value input), // we'll return the system default value. - console.error((e as Error).message); + console.debug((e as Error).message); return defaultValue; } } diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts index fdb0e762d..90a24d5e3 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts @@ -1,4 +1,5 @@ import { InputProps, InputType } from '../../types'; +import { getHelperForInput } from '../getHelperForInput'; import { inputToLiteral, literalToInputValue, @@ -54,8 +55,16 @@ describe('literalToInputValue', () => { ); }); - // TODO - it('should return system default if parsing literal fails', () => {}); + it('should return system default if parsing literal fails', () => { + const { defaultValue } = getHelperForInput(InputType.Boolean); + expect( + literalToInputValue( + { type: InputType.Boolean }, + // Invalid boolean input value because it uses the string field + { scalar: { primitive: { stringValue: 'whoops' } } } + ) + ).toEqual(defaultValue); + }); }); describe('inputToLiteral', () => { From 4907fff99ee7c3530cf3bdc88e6e453db97c2349 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Thu, 9 Jan 2020 18:01:57 -0800 Subject: [PATCH 05/10] Implementing parsing of default values for collections and handling noneType values correctly --- .../inputHelpers/collection.ts | 37 +++++++-- .../inputHelpers/inputHelpers.ts | 6 +- .../inputHelpers/test/inputHelpers.test.ts | 75 +++++++++++++++++-- .../inputHelpers/test/testCases.ts | 3 +- .../LaunchWorkflowForm/inputHelpers/types.ts | 3 +- 5 files changed, 110 insertions(+), 14 deletions(-) diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/collection.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/collection.ts index beab04b32..afd8717c0 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/collection.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/collection.ts @@ -1,10 +1,12 @@ import { Core } from 'flyteidl'; -import { InputValue } from '../types'; +import { InputTypeDefinition, InputValue } from '../types'; import { literalNone } from './constants'; import { getHelperForInput } from './getHelperForInput'; import { parseJSON } from './parseJson'; import { ConverterInput, InputHelper } from './types'; +const missingSubTypeError = 'Unexpected missing subtype for collection'; + function parseCollection(list: string) { const parsed = parseJSON(list); if (!Array.isArray(parsed)) { @@ -13,9 +15,34 @@ function parseCollection(list: string) { return parsed; } -function fromLiteral(literal: Core.ILiteral): InputValue { - // TODO - return ''; +function fromLiteral( + literal: Core.ILiteral, + { subtype }: InputTypeDefinition +): InputValue { + if (!subtype) { + throw new Error(missingSubTypeError); + } + if (!literal.collection) { + throw new Error('Collection literal missing `collection` property'); + } + if (!literal.collection.literals) { + throw new Error( + 'Collection literal missing `colleciton.literals` property' + ); + } + + const subTypeHelper = getHelperForInput(subtype.type); + const values = literal.collection.literals.reduce( + (out, literal) => { + const value = subTypeHelper.fromLiteral(literal, subtype); + if (value !== undefined) { + out.push(value.toString()); + } + return out; + }, + [] + ); + return `[${values.join(',')}]`; } function toLiteral({ @@ -23,7 +50,7 @@ function toLiteral({ typeDefinition: { subtype } }: ConverterInput): Core.ILiteral { if (!subtype) { - throw new Error('Unexpected missing subtype for collection'); + throw new Error(missingSubTypeError); } let parsed: any[]; // If we're processing a nested collection, it may already have been parsed diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts index 11ebc4631..2bd15ca65 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts @@ -30,8 +30,12 @@ export function literalToInputValue( typeDefinition.type ); + if (literal.scalar && literal.scalar.noneType) { + return undefined; + } + try { - return fromLiteral(literal); + return fromLiteral(literal, typeDefinition); } catch (e) { // If something goes wrong (most likely malformed default value input), // we'll return the system default value. diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts index 90a24d5e3..12adbc91e 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts @@ -1,4 +1,6 @@ +import { Core } from 'flyteidl'; import { InputProps, InputType } from '../../types'; +import { literalNone } from '../constants'; import { getHelperForInput } from '../getHelperForInput'; import { inputToLiteral, @@ -46,13 +48,76 @@ function makeNestedCollectionInput(type: InputType, value: string): InputProps { describe('literalToInputValue', () => { describe('Primitives', () => { literalToInputTestCases.map(([type, input, output]) => - it(`Should correctly convert ${type}: ${JSON.stringify( + it(`should correctly convert ${type}: ${JSON.stringify( input.scalar!.primitive )}`, () => { const result = literalToInputValue({ type }, input); expect(result).toEqual(output); }) ); + + [ + InputType.Collection, + InputType.Datetime, + InputType.Duration, + InputType.Float, + InputType.Integer, + InputType.String + ].map(type => + it(`should convert None value for ${type} to undefined`, () => { + expect( + literalToInputValue({ type }, literalNone()) + ).toBeUndefined(); + }) + ); + + it('should correctly convert noneType to undefined', () => { + expect( + literalToInputValue({ type: InputType.None }, literalNone()) + ).toEqual(undefined); + }); + }); + + describe('Collections', () => { + literalToInputTestCases.map(([type, input, output]) => + it(`should correctly convert collection of ${type}: ${JSON.stringify( + input.scalar!.primitive + )}`, () => { + const collection: Core.ILiteral = { + collection: { + // Duplicate it to test comma separation + literals: [input, input] + } + }; + const stringifiedValue = + output === undefined ? '' : output.toString(); + const expectedString = `[${stringifiedValue},${stringifiedValue}]`; + const result = literalToInputValue( + { type: InputType.Collection, subtype: { type } }, + collection + ); + expect(result).toEqual(expectedString); + }) + ); + + it('should return empty for noneType literals', () => { + const collection: Core.ILiteral = { + collection: { + // Duplicate it to test comma separation + literals: [literalNone(), literalNone()] + } + }; + + expect( + literalToInputValue( + { + type: InputType.Collection, + subtype: { type: InputType.None } + }, + collection + ) + ).toEqual('[]'); + }); }); it('should return system default if parsing literal fails', () => { @@ -70,7 +135,7 @@ describe('literalToInputValue', () => { describe('inputToLiteral', () => { describe('Primitives', () => { literalTestCases.map(([type, input, output]) => - it(`Should correctly convert ${type}: ${input} (${typeof input})`, () => { + it(`should correctly convert ${type}: ${input} (${typeof input})`, () => { const result = inputToLiteral(makeSimpleInput(type, input)); expect(result.scalar!.primitive).toEqual(output); }) @@ -88,7 +153,7 @@ describe('inputToLiteral', () => { value = `"${input}"`; } - it(`Should correctly convert collection of type ${type}: [${value}] (${typeof input})`, () => { + it(`should correctly convert collection of type ${type}: [${value}] (${typeof input})`, () => { const result = inputToLiteral( makeCollectionInput(type, `[${value}]`) ); @@ -97,7 +162,7 @@ describe('inputToLiteral', () => { ).toEqual(output); }); - it(`Should correctly convert nested collection of type ${type}: [[${value}]] (${typeof input})`, () => { + it(`should correctly convert nested collection of type ${type}: [[${value}]] (${typeof input})`, () => { const result = inputToLiteral( makeNestedCollectionInput(type, `[[${value}]]`) ); @@ -120,7 +185,7 @@ describe('inputToLiteral', () => { InputType.Struct, InputType.Unknown ].map(type => - it(`Should return empty value for type: ${type}`, () => { + it(`should return empty value for type: ${type}`, () => { expect( inputToLiteral(makeSimpleInput(type, '')).scalar ).toEqual({ noneType: {} }); diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/testCases.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/testCases.ts index c5ee8bc42..bab151476 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/testCases.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/testCases.ts @@ -156,6 +156,5 @@ export const literalToInputTestCases: InputToLiteralTestParams[] = [ Long.MIN_VALUE.toString() ], [InputType.String, primitiveLiteral({ stringValue: '' }), ''], - [InputType.String, primitiveLiteral({ stringValue: 'abcdefg' }), 'abcdefg'], - [InputType.None, literalNone(), undefined] + [InputType.String, primitiveLiteral({ stringValue: 'abcdefg' }), 'abcdefg'] ]; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/types.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/types.ts index 514896035..fb69991f9 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/types.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/types.ts @@ -10,7 +10,8 @@ export type InputToLiteralConverterFn = ( input: ConverterInput ) => Core.ILiteral; export type LiteralToInputConterterFn = ( - literal: Core.ILiteral + literal: Core.ILiteral, + typeDefinition: InputTypeDefinition ) => InputValue | undefined; export interface InputHelper { defaultValue?: InputValue; From 0a211863feeef4bc5c584a1ae42953465836b496 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Mon, 13 Jan 2020 11:09:11 -0800 Subject: [PATCH 06/10] Docs --- .../Launch/LaunchWorkflowForm/inputHelpers/boolean.ts | 4 ++-- .../LaunchWorkflowForm/inputHelpers/constants.ts | 5 ++++- .../Launch/LaunchWorkflowForm/inputHelpers/datetime.ts | 4 ++-- .../Launch/LaunchWorkflowForm/inputHelpers/duration.ts | 4 ++-- .../Launch/LaunchWorkflowForm/inputHelpers/float.ts | 4 ++-- .../LaunchWorkflowForm/inputHelpers/inputHelpers.ts | 10 ++++++++++ .../Launch/LaunchWorkflowForm/inputHelpers/integer.ts | 4 ++-- .../Launch/LaunchWorkflowForm/inputHelpers/string.ts | 4 ++-- .../Launch/LaunchWorkflowForm/inputHelpers/utils.ts | 3 +++ 9 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/boolean.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/boolean.ts index 668f47158..5de989e65 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/boolean.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/boolean.ts @@ -1,6 +1,6 @@ import { Core } from 'flyteidl'; import { InputValue } from '../types'; -import { literalValuePaths } from './constants'; +import { primitiveLiteralPaths } from './constants'; import { ConverterInput, InputHelper } from './types'; import { extractLiteralWithCheck } from './utils'; @@ -43,7 +43,7 @@ function toLiteral({ value }: ConverterInput): Core.ILiteral { function fromLiteral(literal: Core.ILiteral): InputValue { return extractLiteralWithCheck( literal, - literalValuePaths.scalarBoolean + primitiveLiteralPaths.scalarBoolean ); } diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/constants.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/constants.ts index 03324b1bd..15aad09e3 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/constants.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/constants.ts @@ -9,7 +9,10 @@ export const allowedDateFormats = [ISO_8601, RFC_2822]; const primitivePath = 'scalar.primitive'; -export const literalValuePaths = { +/** Strings constants which can be used to perform a deep `get` on a scalar + * literal type using a primitive value. + */ +export const primitiveLiteralPaths = { scalarBoolean: `${primitivePath}.boolean`, scalarDatetime: `${primitivePath}.datetime`, scalarDuration: `${primitivePath}.duration`, diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/datetime.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/datetime.ts index f564cd30d..a85f80e20 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/datetime.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/datetime.ts @@ -2,7 +2,7 @@ import { dateToTimestamp, timestampToDate } from 'common/utils'; import { Core, Protobuf } from 'flyteidl'; import { utc as moment } from 'moment'; import { InputValue } from '../types'; -import { allowedDateFormats, literalValuePaths } from './constants'; +import { allowedDateFormats, primitiveLiteralPaths } from './constants'; import { ConverterInput, InputHelper } from './types'; import { extractLiteralWithCheck } from './utils'; @@ -15,7 +15,7 @@ function parseDate(value: InputValue) { function fromLiteral(literal: Core.ILiteral): InputValue { const value = extractLiteralWithCheck( literal, - literalValuePaths.scalarDatetime + primitiveLiteralPaths.scalarDatetime ); return timestampToDate(value); } diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/duration.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/duration.ts index 065ddd80f..8af578678 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/duration.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/duration.ts @@ -1,7 +1,7 @@ import { durationToMilliseconds, millisecondsToDuration } from 'common/utils'; import { Core, Protobuf } from 'flyteidl'; import { InputValue } from '../types'; -import { literalValuePaths } from './constants'; +import { primitiveLiteralPaths } from './constants'; import { isValidFloat } from './float'; import { ConverterInput, InputHelper } from './types'; import { extractLiteralWithCheck } from './utils'; @@ -9,7 +9,7 @@ import { extractLiteralWithCheck } from './utils'; function fromLiteral(literal: Core.ILiteral): InputValue { const value = extractLiteralWithCheck( literal, - literalValuePaths.scalarDuration + primitiveLiteralPaths.scalarDuration ); return durationToMilliseconds(value); } diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/float.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/float.ts index 9dbaf2890..7a3676767 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/float.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/float.ts @@ -1,13 +1,13 @@ import { Core } from 'flyteidl'; import { InputValue } from '../types'; -import { literalValuePaths } from './constants'; +import { primitiveLiteralPaths } from './constants'; import { ConverterInput, InputHelper } from './types'; import { extractLiteralWithCheck } from './utils'; function fromLiteral(literal: Core.ILiteral): InputValue { return extractLiteralWithCheck( literal, - literalValuePaths.scalarFloat + primitiveLiteralPaths.scalarFloat ); } diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts index 2bd15ca65..d87cc27cb 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts @@ -6,6 +6,9 @@ import { literalNone } from './constants'; import { getHelperForInput } from './getHelperForInput'; type ToLiteralParams = Pick; +/** Converts a type/InputValue combination to a `Core.ILiteral` which can be + * submitted to Admin for creating an execution. + */ export function inputToLiteral(input: ToLiteralParams): Core.ILiteral { if (input.value == null) { return literalNone(); @@ -16,12 +19,16 @@ export function inputToLiteral(input: ToLiteralParams): Core.ILiteral { return toLiteral({ value, typeDefinition }); } +/** Generates the default value (if any) for a given type. */ export function defaultValueForInputType( typeDefinition: InputTypeDefinition ): InputValue | undefined { return getHelperForInput(typeDefinition.type).defaultValue; } +/** Converts a Core.ILiteral to an InputValue which can be used to populate + * a form control. + */ export function literalToInputValue( typeDefinition: InputTypeDefinition, literal: Core.ILiteral @@ -45,6 +52,9 @@ export function literalToInputValue( } type ValidationParams = Pick; +/** Validates a given InputValue based on rules for the provided type. Returns + * void if no errors, throws an error otherwise. + */ export function validateInput(input: ValidationParams) { if (input.value == null) { return; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/integer.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/integer.ts index d0f37d804..2a962c167 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/integer.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/integer.ts @@ -1,7 +1,7 @@ import { Core } from 'flyteidl'; import * as Long from 'long'; import { InputValue } from '../types'; -import { literalValuePaths } from './constants'; +import { primitiveLiteralPaths } from './constants'; import { ConverterInput, InputHelper } from './types'; import { extractLiteralWithCheck } from './utils'; @@ -10,7 +10,7 @@ const integerRegexPattern = /^-?[0-9]+$/; function fromLiteral(literal: Core.ILiteral): InputValue { return extractLiteralWithCheck( literal, - literalValuePaths.scalarInteger + primitiveLiteralPaths.scalarInteger ).toString(); } diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/string.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/string.ts index a56cc9bac..46364e8bf 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/string.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/string.ts @@ -1,13 +1,13 @@ import { Core } from 'flyteidl'; import { InputValue } from '../types'; -import { literalValuePaths } from './constants'; +import { primitiveLiteralPaths } from './constants'; import { ConverterInput, InputHelper } from './types'; import { extractLiteralWithCheck } from './utils'; function fromLiteral(literal: Core.ILiteral): InputValue { return extractLiteralWithCheck( literal, - literalValuePaths.scalarString + primitiveLiteralPaths.scalarString ); } diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/utils.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/utils.ts index 075bf139e..ace48caa9 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/utils.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/utils.ts @@ -1,6 +1,9 @@ import { Core } from 'flyteidl'; import { get } from 'lodash'; +/** Performs a deep get of `path` on the given `Core.ILiteral`. Will throw + * if the given property doesn't exist. + */ export function extractLiteralWithCheck( literal: Core.ILiteral, path: string From 5ad1863b4981db257c2fd094c54cb0a54657cc27 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Mon, 13 Jan 2020 11:50:29 -0800 Subject: [PATCH 07/10] Render asterisks for required inputs --- .../__mocks__/mockInputs.ts | 7 +---- .../LaunchWorkflowForm.stories.tsx | 26 ++++++++++++++++--- .../test/LaunchWorkflowForm.test.tsx | 15 +++++++++++ .../useLaunchWorkflowFormState.ts | 3 ++- 4 files changed, 40 insertions(+), 11 deletions(-) diff --git a/src/components/Launch/LaunchWorkflowForm/__mocks__/mockInputs.ts b/src/components/Launch/LaunchWorkflowForm/__mocks__/mockInputs.ts index 13f54400e..96d44a75d 100644 --- a/src/components/Launch/LaunchWorkflowForm/__mocks__/mockInputs.ts +++ b/src/components/Launch/LaunchWorkflowForm/__mocks__/mockInputs.ts @@ -1,10 +1,5 @@ import { mapValues } from 'lodash'; -import { - ParameterMap, - SimpleType, - TypedInterface, - Variable -} from 'models/Common'; +import { SimpleType, TypedInterface, Variable } from 'models/Common'; function simpleType(primitiveType: SimpleType, description?: string): Variable { return { diff --git a/src/components/Launch/LaunchWorkflowForm/__stories__/LaunchWorkflowForm.stories.tsx b/src/components/Launch/LaunchWorkflowForm/__stories__/LaunchWorkflowForm.stories.tsx index cc70310aa..83415e94b 100644 --- a/src/components/Launch/LaunchWorkflowForm/__stories__/LaunchWorkflowForm.stories.tsx +++ b/src/components/Launch/LaunchWorkflowForm/__stories__/LaunchWorkflowForm.stories.tsx @@ -20,9 +20,11 @@ import { } from '../__mocks__/mockInputs'; import { LaunchWorkflowForm } from '../LaunchWorkflowForm'; +const stringInputName = 'simpleString'; +const integerInputName = 'simpleInteger'; const submitAction = action('createWorkflowExecution'); -const renderForm = (variables: Record) => { +const generateMocks = (variables: Record) => { const mockWorkflow = createMockWorkflow('MyWorkflow'); const mockLaunchPlan = createMockLaunchPlan( mockWorkflow.id.name, @@ -63,6 +65,13 @@ const renderForm = (variables: Record) => { listLaunchPlans: () => resolveAfter(500, { entities: [mockLaunchPlan] }) }); + return { mockWorkflow, mockLaunchPlan, mockWorkflowVersions, mockApi }; +}; + +const renderForm = ({ + mockApi, + mockWorkflow +}: ReturnType) => { const onClose = () => console.log('Close'); return ( @@ -79,8 +88,17 @@ const renderForm = (variables: Record) => { const stories = storiesOf('Launch/LaunchWorkflowForm', module); -stories.add('Simple', () => renderForm(mockSimpleVariables)); -stories.add('Collections', () => renderForm(mockCollectionVariables)); +stories.add('Simple', () => renderForm(generateMocks(mockSimpleVariables))); +stories.add('Required Inputs', () => { + const mocks = generateMocks(mockSimpleVariables); + const parameters = mocks.mockLaunchPlan.closure!.expectedInputs.parameters; + parameters[stringInputName].required = true; + parameters[integerInputName].required = true; + return renderForm(mocks); +}); +stories.add('Collections', () => + renderForm(generateMocks(mockCollectionVariables)) +); stories.add('Nested Collections', () => - renderForm(mockNestedCollectionVariables) + renderForm(generateMocks(mockNestedCollectionVariables)) ); diff --git a/src/components/Launch/LaunchWorkflowForm/test/LaunchWorkflowForm.test.tsx b/src/components/Launch/LaunchWorkflowForm/test/LaunchWorkflowForm.test.tsx index 1a4f9613a..86bc00eb2 100644 --- a/src/components/Launch/LaunchWorkflowForm/test/LaunchWorkflowForm.test.tsx +++ b/src/components/Launch/LaunchWorkflowForm/test/LaunchWorkflowForm.test.tsx @@ -426,6 +426,21 @@ describe('LaunchWorkflowForm', () => { getByLabelText(integerInputName, { exact: false }) ).toHaveValue('10000'); }); + + it('should decorate labels for required inputs', async () => { + // Add defaults for the string/integer inputs and check that they are + // correctly populated + const parameters = mockLaunchPlans[0].closure!.expectedInputs + .parameters; + parameters[stringInputName].required = true; + mockGetLaunchPlan.mockResolvedValue(mockLaunchPlans[0]); + + const { getByText } = renderForm(); + await wait(); + expect( + getByText(stringInputName, { exact: false }).textContent + ).toContain('*'); + }); }); }); }); diff --git a/src/components/Launch/LaunchWorkflowForm/useLaunchWorkflowFormState.ts b/src/components/Launch/LaunchWorkflowForm/useLaunchWorkflowFormState.ts index 38431a96d..708fb99ce 100644 --- a/src/components/Launch/LaunchWorkflowForm/useLaunchWorkflowFormState.ts +++ b/src/components/Launch/LaunchWorkflowForm/useLaunchWorkflowFormState.ts @@ -61,7 +61,8 @@ function getInputs(workflow: Workflow, launchPlan: LaunchPlan): ParsedInput[] { const typeDefinition = getInputDefintionForLiteralType( parameter.var.type ); - const label = formatLabelWithType(name, typeDefinition); + const typeLabel = formatLabelWithType(name, typeDefinition); + const label = required ? `${typeLabel}*` : typeLabel; const defaultValue = parameter.default !== undefined From 4c6083737c3dba58ffb703d336d533703562a3a7 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Mon, 13 Jan 2020 11:58:20 -0800 Subject: [PATCH 08/10] Adding validation for required fields --- .../LaunchWorkflowForm/inputHelpers/inputHelpers.ts | 8 +++++++- .../inputHelpers/test/inputHelpers.test.ts | 8 ++++++++ .../Launch/LaunchWorkflowForm/useFormInputsState.ts | 4 ++-- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts index d87cc27cb..2c9a9531f 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts @@ -51,12 +51,18 @@ export function literalToInputValue( } } -type ValidationParams = Pick; +type ValidationParams = Pick< + InputProps, + 'name' | 'required' | 'typeDefinition' | 'value' +>; /** Validates a given InputValue based on rules for the provided type. Returns * void if no errors, throws an error otherwise. */ export function validateInput(input: ValidationParams) { if (input.value == null) { + if (input.required) { + throw new ValueError(input.name, 'Value is required'); + } return; } diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts index 12adbc91e..2fe8bcb98 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts @@ -235,4 +235,12 @@ describe('validateInput', () => { describe('string', () => { generateValidityTests(InputType.String, validityTestCases.string); }); + + it('should throw errors for missing required values', () => { + const [type, input] = literalTestCases[0]; + const simpleInput = makeSimpleInput(type, input); + simpleInput.required = true; + delete simpleInput.value; + expect(() => validateInput(simpleInput)).toThrowError(); + }); }); diff --git a/src/components/Launch/LaunchWorkflowForm/useFormInputsState.ts b/src/components/Launch/LaunchWorkflowForm/useFormInputsState.ts index a048b756f..8646374fb 100644 --- a/src/components/Launch/LaunchWorkflowForm/useFormInputsState.ts +++ b/src/components/Launch/LaunchWorkflowForm/useFormInputsState.ts @@ -26,9 +26,9 @@ function useFormInputState(parsedInput: ParsedInput): FormInputState { const validationValue = useDebouncedValue(value, debounceDelay); const validate = () => { - const { name, typeDefinition } = parsedInput; + const { name, required, typeDefinition } = parsedInput; try { - validateInput({ name, typeDefinition, value }); + validateInput({ name, required, typeDefinition, value }); setError(undefined); return true; } catch (e) { From 314cf5ce329d94628c4f6c6ac1536cfe13ac0a2a Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Mon, 13 Jan 2020 14:05:03 -0800 Subject: [PATCH 09/10] Adding stories for default values and fixing default value parsing for dates --- .../__mocks__/mockInputs.ts | 41 ++++++++++++++++++- .../LaunchWorkflowForm/__mocks__/utils.ts | 5 +++ .../LaunchWorkflowForm.stories.tsx | 16 +++++++- .../inputHelpers/datetime.ts | 2 +- .../inputHelpers/test/testCases.ts | 8 +--- 5 files changed, 62 insertions(+), 10 deletions(-) create mode 100644 src/components/Launch/LaunchWorkflowForm/__mocks__/utils.ts diff --git a/src/components/Launch/LaunchWorkflowForm/__mocks__/mockInputs.ts b/src/components/Launch/LaunchWorkflowForm/__mocks__/mockInputs.ts index 96d44a75d..8f1a67116 100644 --- a/src/components/Launch/LaunchWorkflowForm/__mocks__/mockInputs.ts +++ b/src/components/Launch/LaunchWorkflowForm/__mocks__/mockInputs.ts @@ -1,5 +1,10 @@ +import { dateToTimestamp, millisecondsToDuration } from 'common/utils'; +import { Core } from 'flyteidl'; import { mapValues } from 'lodash'; +import * as Long from 'long'; import { SimpleType, TypedInterface, Variable } from 'models/Common'; +import { literalNone } from '../inputHelpers/constants'; +import { primitiveLiteral } from './utils'; function simpleType(primitiveType: SimpleType, description?: string): Variable { return { @@ -10,7 +15,21 @@ function simpleType(primitiveType: SimpleType, description?: string): Variable { }; } -export const mockSimpleVariables: Record = { +const validDateString = '2019-01-10T00:00:00.000Z'; // Dec 1, 2019 + +export type SimpleVariableKey = + | 'simpleString' + | 'stringNoLabel' + | 'simpleInteger' + | 'simpleFloat' + | 'simpleBoolean' + | 'simpleDuration' + | 'simpleDatetime' + | 'simpleBinary' + | 'simpleError' + | 'simpleStruct'; + +export const mockSimpleVariables: Record = { simpleString: simpleType(SimpleType.STRING, 'a simple string value'), stringNoLabel: simpleType(SimpleType.STRING), simpleInteger: simpleType(SimpleType.INTEGER, 'a simple integer value'), @@ -27,6 +46,26 @@ export const mockSimpleVariables: Record = { // blob: {} }; +export const simpleVariableDefaults: Record< + SimpleVariableKey, + Core.ILiteral +> = { + simpleString: primitiveLiteral({ stringValue: 'abcdefg' }), + stringNoLabel: primitiveLiteral({ stringValue: 'abcdefg' }), + simpleBinary: literalNone(), + simpleBoolean: primitiveLiteral({ boolean: false }), + simpleDatetime: primitiveLiteral({ + datetime: dateToTimestamp(new Date(validDateString)) + }), + simpleDuration: primitiveLiteral({ + duration: millisecondsToDuration(10000) + }), + simpleError: literalNone(), + simpleFloat: primitiveLiteral({ floatValue: 1.5 }), + simpleInteger: primitiveLiteral({ integer: Long.fromNumber(12345) }), + simpleStruct: literalNone() +}; + export const mockCollectionVariables: Record = mapValues( mockSimpleVariables, v => ({ diff --git a/src/components/Launch/LaunchWorkflowForm/__mocks__/utils.ts b/src/components/Launch/LaunchWorkflowForm/__mocks__/utils.ts new file mode 100644 index 000000000..6584ea8a0 --- /dev/null +++ b/src/components/Launch/LaunchWorkflowForm/__mocks__/utils.ts @@ -0,0 +1,5 @@ +import { Core } from 'flyteidl'; + +export function primitiveLiteral(primitive: Core.IPrimitive): Core.ILiteral { + return { scalar: { primitive } }; +} diff --git a/src/components/Launch/LaunchWorkflowForm/__stories__/LaunchWorkflowForm.stories.tsx b/src/components/Launch/LaunchWorkflowForm/__stories__/LaunchWorkflowForm.stories.tsx index 83415e94b..d279305c7 100644 --- a/src/components/Launch/LaunchWorkflowForm/__stories__/LaunchWorkflowForm.stories.tsx +++ b/src/components/Launch/LaunchWorkflowForm/__stories__/LaunchWorkflowForm.stories.tsx @@ -4,7 +4,7 @@ import { resolveAfter } from 'common/promiseUtils'; import { mockAPIContextValue } from 'components/data/__mocks__/apiContext'; import { APIContext } from 'components/data/apiContext'; import { mapValues } from 'lodash'; -import { Variable, Workflow } from 'models'; +import { Literal, Variable, Workflow } from 'models'; import { createMockLaunchPlan } from 'models/__mocks__/launchPlanData'; import { createMockWorkflow, @@ -16,7 +16,9 @@ import { createMockWorkflowInputsInterface, mockCollectionVariables, mockNestedCollectionVariables, - mockSimpleVariables + mockSimpleVariables, + simpleVariableDefaults, + SimpleVariableKey } from '../__mocks__/mockInputs'; import { LaunchWorkflowForm } from '../LaunchWorkflowForm'; @@ -96,6 +98,16 @@ stories.add('Required Inputs', () => { parameters[integerInputName].required = true; return renderForm(mocks); }); +stories.add('Default Values', () => { + const mocks = generateMocks(mockSimpleVariables); + const parameters = mocks.mockLaunchPlan.closure!.expectedInputs.parameters; + Object.keys(parameters).forEach(paramName => { + const defaultValue = + simpleVariableDefaults[paramName as SimpleVariableKey]; + parameters[paramName].default = defaultValue as Literal; + }); + return renderForm(mocks); +}); stories.add('Collections', () => renderForm(generateMocks(mockCollectionVariables)) ); diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/datetime.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/datetime.ts index a85f80e20..f1c5a1998 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/datetime.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/datetime.ts @@ -17,7 +17,7 @@ function fromLiteral(literal: Core.ILiteral): InputValue { literal, primitiveLiteralPaths.scalarDatetime ); - return timestampToDate(value); + return timestampToDate(value).toISOString(); } function toLiteral({ value }: ConverterInput): Core.ILiteral { diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/testCases.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/testCases.ts index bab151476..8524c21c3 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/testCases.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/testCases.ts @@ -1,8 +1,8 @@ import { dateToTimestamp, millisecondsToDuration } from 'common/utils'; import { Core } from 'flyteidl'; import * as Long from 'long'; +import { primitiveLiteral } from '../../__mocks__/utils'; import { InputType, InputValue } from '../../types'; -import { literalNone } from '../constants'; // Defines type of value, input, and expected value of innermost `IScalar` type PrimitiveTestParams = [InputType, any, Core.IPrimitive]; @@ -99,10 +99,6 @@ export const literalTestCases: PrimitiveTestParams[] = [ [InputType.String, 'abcdefg', { stringValue: 'abcdefg' }] ]; -function primitiveLiteral(primitive: Core.IPrimitive): Core.ILiteral { - return { scalar: { primitive } }; -} - type InputToLiteralTestParams = [ InputType, Core.ILiteral, @@ -116,7 +112,7 @@ export const literalToInputTestCases: InputToLiteralTestParams[] = [ primitiveLiteral({ datetime: dateToTimestamp(new Date(validDateString)) }), - new Date(validDateString) + validDateString ], [ InputType.Duration, From f4450b80f3356453e5d31a2cd01cb2e5ba728bcb Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Mon, 13 Jan 2020 14:21:15 -0800 Subject: [PATCH 10/10] Default parameters do not imply required values --- .../__stories__/LaunchWorkflowForm.stories.tsx | 2 ++ .../Launch/LaunchWorkflowForm/useLaunchWorkflowFormState.ts | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/Launch/LaunchWorkflowForm/__stories__/LaunchWorkflowForm.stories.tsx b/src/components/Launch/LaunchWorkflowForm/__stories__/LaunchWorkflowForm.stories.tsx index d279305c7..a6e64c383 100644 --- a/src/components/Launch/LaunchWorkflowForm/__stories__/LaunchWorkflowForm.stories.tsx +++ b/src/components/Launch/LaunchWorkflowForm/__stories__/LaunchWorkflowForm.stories.tsx @@ -22,6 +22,7 @@ import { } from '../__mocks__/mockInputs'; import { LaunchWorkflowForm } from '../LaunchWorkflowForm'; +const booleanInputName = 'simpleBoolean'; const stringInputName = 'simpleString'; const integerInputName = 'simpleInteger'; const submitAction = action('createWorkflowExecution'); @@ -96,6 +97,7 @@ stories.add('Required Inputs', () => { const parameters = mocks.mockLaunchPlan.closure!.expectedInputs.parameters; parameters[stringInputName].required = true; parameters[integerInputName].required = true; + parameters[booleanInputName].required = true; return renderForm(mocks); }); stories.add('Default Values', () => { diff --git a/src/components/Launch/LaunchWorkflowForm/useLaunchWorkflowFormState.ts b/src/components/Launch/LaunchWorkflowForm/useLaunchWorkflowFormState.ts index 708fb99ce..b5a16bcaf 100644 --- a/src/components/Launch/LaunchWorkflowForm/useLaunchWorkflowFormState.ts +++ b/src/components/Launch/LaunchWorkflowForm/useLaunchWorkflowFormState.ts @@ -51,7 +51,7 @@ function getInputs(workflow: Workflow, launchPlan: LaunchPlan): ParsedInput[] { const launchPlanInputs = launchPlan.closure.expectedInputs.parameters; return sortedObjectEntries(launchPlanInputs).map(value => { const [name, parameter] = value; - const required = !!(parameter.default || parameter.required); + const required = !!parameter.required; const workflowInput = workflowInputs[name]; const description = workflowInput && workflowInput.description