add support for creating camel source #4960

nemesis09 · 2020-04-08T04:41:50Z

Fixes:
https://issues.redhat.com/browse/ODC-2661

Analysis / Root cause:
User can't create CamelSource via webConsole

Solution Description:
Add support for creation of Camel Source to the Event Source Form in the Add flow

Screens:

Unit Test Coverage:

Browser Conformance:

Firefox
Chrome
Safari
Edge

invincibleJai · 2020-04-08T04:55:43Z

/assign @rohitkrai03

rohitkrai03 · 2020-04-08T10:39:50Z

frontend/packages/knative-plugin/src/components/add/EventSource.tsx

  };

  const createResources = (rawFormData: any): Promise<K8sResourceKind> => {
-    const knEventSourceResource = getEventSourcesDepResource(rawFormData);
+    let knEventSourceResource = getEventSourcesDepResource(rawFormData);


Put the assignment into the else block. That way when the editor type is YAML, only one of the function is called to get the resource data.

rohitkrai03 · 2020-04-08T12:08:25Z

frontend/packages/knative-plugin/src/components/add/EventSource.tsx

-    const knEventSourceResource = getEventSourcesDepResource(rawFormData);
+    let knEventSourceResource = getEventSourcesDepResource(rawFormData);
+    if (rawFormData.editorType === EditorType.YAML) {
+      knEventSourceResource = getEventSourcesResource(rawFormData);


I am not sure what is the difference between getEventSourcesResource and getEventSourcesDepResource going by just the name of the functions. Better to name it in a way that gives more info upfront.

changed the function name to getEventSourcesResourceFromYAML

rohitkrai03 · 2020-04-08T12:10:49Z

frontend/packages/knative-plugin/src/components/add/event-sources/CamelSection.tsx

+import YAMLEditorField from '@console/shared/src/components/formik-fields/YAMLEditorField';
+import FormSection from '@console/dev-console/src/components/import/section/FormSection';
+
+const CamelSection: React.FC = () => {


Why is it specifically named CamelSection? I think this should be a generic YAMLSection where you take title as a prop based on the event source type. This should also work for dynamic event sources.

created YAMLEditorSection to take title from formik values

rohitkrai03 · 2020-04-08T12:12:51Z

frontend/packages/knative-plugin/src/components/add/EventSourceForm.tsx

+      {values.type === EventSources.CronJobSource && <CronJobSection />}
+      {values.type === EventSources.SinkBinding && <SinkBindingSection />}
+      {values.type === EventSources.ApiServerSource && <ApiServerSection namespace={namespace} />}
+      {values.type === EventSources.CamelSource && <CamelSection />}


You don't need to check specifically for CamelSource. Just create a white list of all the event sources that have their own section, if the current event source type is in that list render their respective sections else just render default YAMLSection.

I would even suggest to move all the logic of checking the event sources into a simple util function renderEventSoiurceSection that has switch case. The default case of that switch returns YAMLSection.

rohitkrai03 · 2020-04-08T12:15:46Z

frontend/packages/knative-plugin/src/components/add/EventSourceForm.tsx

+      {values.type === EventSources.SinkBinding && <SinkBindingSection />}
+      {values.type === EventSources.ApiServerSource && <ApiServerSection namespace={namespace} />}
+      {values.type === EventSources.CamelSource && <CamelSection />}
+      {values.editorType !== EditorType.YAML && <SinkSection namespace={namespace} />}


You don't need to have this new property called editorType in values. You can simply inverse the above logic and render the contents only when the event source selected is in that white list.

I kept the editor type property as to use in validation, as I needed to change the validation schema based on if it is a form or a yaml editor

@nemesis09 validation Schema can be handled based on type right?

something like

const ownEvenstSources = Object.keys(EventSources); export const eventSourceValidationSchema = yup.object().shape({ project: yup.object().when('type', { is: (type) => ownEvenstSources.includes(type), then: projectNameValidationSchema, }), ...... })

and just handle it based on type here as well as @rohitkrai03 commented remove camelSource from EventSources enum i.e if type is not in enum show yamlEditor

rohitkrai03 · 2020-04-08T12:18:51Z

frontend/packages/knative-plugin/src/components/add/event-sources/CamelSection.tsx

+const CamelSection: React.FC = () => {
+  return (
+    <FormSection fullWidth title="CamelSource">
+      <YAMLEditorField name={'yamlData'} />


You don't need to pass a string inside curly braces.

Suggested change

<YAMLEditorField name={'yamlData'} />

<YAMLEditorField name="yamlData" />

rohitkrai03 · 2020-04-08T12:24:09Z

frontend/packages/knative-plugin/src/components/add/import-types.ts

  ApiServerSource = 'ApiServerSource',
  KafkaSource = 'KafkaSource',
-  CamelkSource = 'CamelSource',
+  CamelSource = 'CamelSource',


You can just remove this line from the enum and you have your list of event sources that have their own form sections.

rohitkrai03 · 2020-04-08T12:24:40Z

frontend/packages/knative-plugin/src/components/add/import-types.ts

@@ -34,6 +33,8 @@ export interface EventSourceFormData {
  type: string;
  sink: KnativeServiceName;
  data?: EventSourceData;
+  yamlData?: string;
+  editorType?: string;


Suggested change

editorType?: string;

editorType?: EditorType;

christianvogt · 2020-04-08T12:32:41Z

frontend/packages/knative-plugin/src/components/add/EventSourceForm.tsx

-  </Form>
-);
+}) => {
+  return (


I see this else where in your pr too. Please address all. Prefer immediate return values with no return statement as was present before your change.

I still see that you have explicit return instead of directly returning the JSX.

christianvogt · 2020-04-08T12:35:40Z

frontend/packages/knative-plugin/src/components/add/event-sources/EventSourcesSelector.tsx

  const handleItemChange = React.useCallback(
    (item: string) => {
      const name = `data.${item.toLowerCase()}`;
      setFieldValue(name, getEventSourceData(item.toLowerCase()));
      setFieldTouched(name, true);
+      setFieldValue('editorType', EditorType.Form);
+      setFieldTouched('editorType', true);
+      if (item.toLowerCase() === 'camelsource') {


This seems setting to be handling specific logic for a specific type here in this genetic component.

changed it to check and load form or yaml editor values for any event source selected

christianvogt · 2020-04-08T12:38:37Z

frontend/packages/knative-plugin/src/utils/create-eventsources-utils.ts

+  eventSource: string,
+): K8sResourceKind => {
+  let eventSourceModel: K8sKind;
+  const name = `${eventSource}-test`;


Why is there a name with test here?

changed the name to ${eventSource}

christianvogt · 2020-04-08T12:39:04Z

frontend/packages/knative-plugin/src/components/add/EventSource.tsx

 } from '../../utils/create-eventsources-utils';
+import * as _ from 'lodash';


Absolute imports first

invincibleJai · 2020-04-09T10:33:54Z

frontend/packages/knative-plugin/src/components/add/event-sources/EventSourcesSelector.tsx

+        setFieldValue('editorType', EditorType.Form);
+        setFieldTouched('editorType', true);


we can avoid this field itself editorType if we rely on type

invincibleJai · 2020-04-09T10:38:18Z

frontend/packages/knative-plugin/src/components/add/event-sources/EventSourcesSelector.tsx


 interface EventSourcesSelectorProps {
  eventSourceList: NormalizedEventSources;
 }

 const EventSourcesSelector: React.FC<EventSourcesSelectorProps> = ({ eventSourceList }) => {
  const { setFieldValue, setFieldTouched, validateForm } = useFormikContext<FormikValues>();
+  const [namespaceField] = useField('project.name');


I think we shouldn't have it here , maybe it's good to introduce a prop on this component onChange and handle these logic in callback in eventSourceForm and access from values

invincibleJai · 2020-04-09T10:38:34Z

frontend/packages/knative-plugin/src/components/add/event-sources/EventSourcesSelector.tsx

-      const name = `data.${item.toLowerCase()}`;
-      setFieldValue(name, getEventSourceData(item.toLowerCase()));
-      setFieldTouched(name, true);
+      if (isKnownEventSourceSelected(item)) {


I think we shouldn't have it here , maybe it's good to introduce a prop on this component onChange and handle these logic in callback in eventSourceForm

invincibleJai · 2020-04-09T10:38:48Z

frontend/packages/knative-plugin/src/components/add/event-sources/EventSourcesSelector.tsx

+        setFieldValue(
+          'yamlData',
+          safeDump(getEventSourceYaml(namespaceField.value, item.toLowerCase())),
+        );


same as above , I think we shouldn't have it here , maybe it's good to introduce a prop on this component onChange and handle these logic in callback in eventSourceForm and access from values

We can simply move the whole loading yaml logic in YAMLSection itself. Just wrap this logic inside a useEffect with dependency on values.type. It will load the YAML if the section is rendered and when the type changes automatically.

moved the logic to eventSourceForm and passed down to EventSourceSelector as a prop.
it handles both the form and yaml editor cases.
should we move just the yaml logic and keep the form logic in handleChange ?

invincibleJai · 2020-04-09T10:41:00Z

frontend/packages/knative-plugin/src/components/add/eventSource-validation-utils.ts

+  project: yup.object().when('editorType', {
+    is: EditorType.Form,
+    then: projectNameValidationSchema,
+  }),
+  application: yup.object().when('editorType', {
+    is: EditorType.Form,
+    then: applicationNameValidationSchema,
+  }),
+  name: yup.string().when('editorType', {


can have validation based on type itself and remove editorType field WDYT?

const ownEvenstSources = Object.keys(EventSources); export const eventSourceValidationSchema = yup.object().shape({ project: yup.object().when('type', { is: (type) => ownEvenstSources.includes(type), then: projectNameValidationSchema, }), ..... })

Or simply add .when to the object and add all the properties based on that one check.

Suggested change

project: yup.object().when('editorType', {

is: EditorType.Form,

then: projectNameValidationSchema,

}),

application: yup.object().when('editorType', {

is: EditorType.Form,

then: applicationNameValidationSchema,

}),

name: yup.string().when('editorType', {

export const eventSourceValidationSchema = yup.object().when('type', {

is: (type) => Object.keys(EventSources).includes(type),

then: yup.object().shape({

project: projectNameValidationSchema,

application: applicationNameValidationSchema,

name: nameValidationSchema,

sink: sinkServiceSchema,

data: sourceDataSpecSchema,

}),

otherwise: yup.object().shape({

data: sourceDataSpecSchema,

}),

});

is: (type) => Object.keys(EventSources).includes(type), was throwing error with this message - type is not a function
approached it using yup.lazy(). PTAL

invincibleJai · 2020-04-09T10:41:45Z

frontend/packages/knative-plugin/src/components/add/import-types.ts

+export enum EditorType {
+  YAML = 'YAMLEditor',
+  Form = 'FormEditor',
+}
+


if use type for conditional validation schema , we may not need this as well

invincibleJai · 2020-04-09T10:52:03Z

frontend/packages/knative-plugin/src/components/add/EventSource.tsx

+    if (isKnownEventSourceSelected(rawFormData.type)) {
+      knEventSourceResource = getEventSourcesDepResource(rawFormData);
+    } else {
+      knEventSourceResource = getEventSourcesResourceFromYAML(rawFormData);


Do we need getEventSourcesResourceFromYAML ?

if (isKnownEventSourceSelected(rawFormData.type)) { knEventSourceResource = getEventSourcesDepResource(rawFormData); } else { knEventSourceResource = safeLoad(rawFormData.chartValuesYAML); }

getEventSourcesResourceFromYAML adds labels and annotations to the yamldata and returns the updated yaml.
safeLoad(rawFormData.yamlData) would load the sample yaml for the event source, without labels and annotations
I updated the function getEventSourcesDepResource to handle both and moved the logic for handling yamldata and formdata to separate functions. PTAL

invincibleJai · 2020-04-09T10:54:32Z

frontend/packages/knative-plugin/src/utils/create-eventsources-utils.ts

+export const getEventSourcesResourceFromYAML = (formData: any): K8sResourceKind => {
+  const {
+    application: { name: applicationName },
+    yamlData,
+  } = formData;
+  const eventYaml = safeLoad(yamlData);
+  const { name } = eventYaml.metadata;
+  const defaultLabel = getAppLabels(name, applicationName);
+  const { apiVersion, kind, spec, metadata } = eventYaml;
+  const eventSourceResource: K8sResourceKind = {
+    apiVersion,
+    kind,
+    metadata: {
+      ...metadata,
+      labels: {
+        ...defaultLabel,
+      },
+      annotations,
+    },
+    spec: {
+      ...spec,
+    },
+  };
+  return eventSourceResource;
+};


can avoid it based on #4960 (comment)

PTAL #4960 (comment)

invincibleJai · 2020-04-09T10:56:20Z

frontend/packages/knative-plugin/src/components/add/event-sources/YAMLEditorSection.tsx

+  const [field] = useField('type');
+  return (
+    <FormSection fullWidth title={field.value}>
+      <YAMLEditorField name="yamlData" />


it will be good have onSave here as well to validate yaml as and when it gets Saved

<YAMLEditorField name="yamlData" onSave={handleSubmit} />

FYI, this onSave is triggered when CTRL+S or CMD+S is pressed by the user. If we don't want that key binding to submit the form in case there are still some form fields left then we should not add this.

invincibleJai · 2020-04-09T10:57:53Z

frontend/packages/knative-plugin/src/utils/create-eventsources-utils.ts

+export const getEventSourceYaml = (namespace: string, eventSource: string): K8sResourceKind => {
+  let eventSourceModel: K8sKind;
+  const name = `${eventSource}`;
+  switch (eventSource) {
+    case 'camelsource':
+      eventSourceModel = EventSourceCamelModel;
+      break;
+    default:
+      eventSourceModel = {
+        apiGroup: KNATIVE_EVENT_SOURCE_APIGROUP_DEP,
+        apiVersion: 'v1alpha1',
+        kind: '',
+        label: '',
+        labelPlural: '',
+        plural: '',
+        abbr: '',
+      };
+  }
+  return {
+    apiVersion: `${eventSourceModel.apiGroup}/${eventSourceModel.apiVersion}`,
+    kind: eventSourceModel.kind,
+    metadata: {
+      name,
+      namespace,
+    },
+    spec: {
+      sink: {


can we make use of getEventSourcesDepResource itself instead of this method

updated the code for getEventSourcesDepResource. PTAL

invincibleJai · 2020-04-09T11:21:06Z

/kind feature

rohitkrai03 · 2020-04-09T11:09:19Z

frontend/packages/knative-plugin/src/components/add/event-sources/EventSourcesSelector.tsx

+        setFieldValue(
+          'yamlData',
+          safeDump(getEventSourceYaml(namespaceField.value, item.toLowerCase())),
+        );


We can simply move the whole loading yaml logic in YAMLSection itself. Just wrap this logic inside a useEffect with dependency on values.type. It will load the YAML if the section is rendered and when the type changes automatically.

rohitkrai03 · 2020-04-09T11:12:06Z

frontend/packages/knative-plugin/src/components/add/event-sources/YAMLEditorSection.tsx

+  const [field] = useField('type');
+  return (
+    <FormSection fullWidth title={field.value}>
+      <YAMLEditorField name="yamlData" />


FYI, this onSave is triggered when CTRL+S or CMD+S is pressed by the user. If we don't want that key binding to submit the form in case there are still some form fields left then we should not add this.

rohitkrai03 · 2020-04-09T11:28:44Z

frontend/packages/knative-plugin/src/components/add/eventSource-validation-utils.ts

+  project: yup.object().when('editorType', {
+    is: EditorType.Form,
+    then: projectNameValidationSchema,
+  }),
+  application: yup.object().when('editorType', {
+    is: EditorType.Form,
+    then: applicationNameValidationSchema,
+  }),
+  name: yup.string().when('editorType', {


Or simply add .when to the object and add all the properties based on that one check.

Suggested change

project: yup.object().when('editorType', {

is: EditorType.Form,

then: projectNameValidationSchema,

}),

application: yup.object().when('editorType', {

is: EditorType.Form,

then: applicationNameValidationSchema,

}),

name: yup.string().when('editorType', {

export const eventSourceValidationSchema = yup.object().when('type', {

is: (type) => Object.keys(EventSources).includes(type),

then: yup.object().shape({

project: projectNameValidationSchema,

application: applicationNameValidationSchema,

name: nameValidationSchema,

sink: sinkServiceSchema,

data: sourceDataSpecSchema,

}),

otherwise: yup.object().shape({

data: sourceDataSpecSchema,

}),

});

invincibleJai · 2020-04-09T20:10:38Z

@nemesis09 Needs rebase

rohitkrai03 · 2020-04-13T10:25:18Z

frontend/packages/knative-plugin/src/components/add/EventSource.tsx

@@ -33,9 +39,10 @@ const EventSource: React.FC<Props> = ({
  activeApplication,
  contextSource,
 }) => {
-  const typeEventSource = EventSources.CronJobSource;
+  const typeEventSource = KnownEventSources.CronJobSource;


Why do you need to rename this enum? This creates unnecessary changes all across the PR.

I thought KnownEventSources should be more descriptive for a name

i think we should have what;s there currently i.e EventSources we can do just this refactor later if need be as in this PR it brings lot of other chnages. WDYT?

Yes. we can change the name if required in a separate PR
reverted to EventSources

rohitkrai03 · 2020-04-13T10:28:08Z

frontend/packages/knative-plugin/src/components/add/EventSourceForm.tsx

+  const EventSourceSection: React.FC = renderEventSourceSection(values.type);
+  const { setFieldValue, setFieldTouched, validateForm } = useFormikContext<FormikValues>();
+  const [namespaceField] = useField('project.name');
+  const handleEventSourceChange = React.useCallback(


Why have you moved this handler out of the selector component and into this parent form component?

moved handler function to EventSourceSelector

rohitkrai03 · 2020-04-13T10:32:16Z

frontend/packages/knative-plugin/src/utils/create-eventsources-utils.ts

+  formData: any,
+  isYamlData?: boolean,
+): K8sResourceKind => {
+  if (isYamlData) {


Why not make use of getEventSourceResource itself to send whatever proper resource data needs to be sent?

updated getEventSourceResource to handle all event sources

rohitkrai03 · 2020-04-13T10:33:27Z

frontend/packages/knative-plugin/src/utils/create-eventsources-utils.ts

+  return eventSourceResource;
+};
+
+const createResourceFromForm = (formData: any): K8sResourceKind => {


This name doesn't make sense here since no actual resource is being created. Its just creating a data structure like the resource which is then used to create actual resource.

renamed to getResourceFromForm

rohitkrai03 · 2020-04-13T10:34:56Z

frontend/packages/knative-plugin/src/components/add/event-sources/YAMLEditorSection.tsx

+import { useField } from 'formik';
+
+const YAMLEditorSection: React.FC = () => {
+  const [field] = useField('type');


It should get the type as a prop and this yaml editor section should act as a field component and take care of loading and updating the yamlData of your form state based on the prop type.

updated YAMLEditorSection to take prop type and accordingly update yamlData

rohitkrai03 · 2020-04-13T10:35:20Z

frontend/packages/knative-plugin/src/components/add/event-sources/EventSourcesSelector.tsx

  const eventSourceItems = Object.keys(eventSourceList).length;
-  const handleItemChange = React.useCallback(


Why have you removed all this logic from here?

rohitkrai03 · 2020-04-13T10:36:13Z

frontend/packages/knative-plugin/src/components/add/EventSource.tsx

-    const knEventSourceResource = getEventSourceResource(rawFormData);
+    const knEventSourceResource = getEventSourcesDepResource(
+      rawFormData,
+      !isKnownEventSourceSelected(rawFormData.type),


Why do you need to do this check here? Why can't this util function do this itself?

updated util function to handle the check

nemesis09 · 2020-04-13T17:47:59Z

/retest

rohitkrai03 · 2020-04-13T18:36:26Z

frontend/packages/knative-plugin/src/components/add/EventSourceForm.tsx

-  </Form>
-);
+}) => {
+  return (


I still see that you have explicit return instead of directly returning the JSX.

rohitkrai03 · 2020-04-13T18:37:22Z

frontend/packages/knative-plugin/src/components/add/EventSourceForm.tsx

+  return (
+    <Form className="co-deploy-image" onSubmit={handleSubmit}>
+      <EventSourcesSelector eventSourceList={useEventSourceList(namespace)} />
+      <AsyncComponent


Why do you need an Async loader here?

updated code to use React.createElement in the util instead

Why are you so keen on having this in utils only? Why not just create an EventSourceSection component which handles this?

rohitkrai03 · 2020-04-13T18:38:30Z

frontend/packages/knative-plugin/src/components/add/EventSourceForm.tsx

+        loader={eventSourceSectionLoader(values.type)}
+        eventSourceType={values.type}
+      />
+      {isKnownEventSourceSelected(values.type) && <SinkSection namespace={namespace} />}


Why do you have two same checks here? Merge them together.

rohitkrai03 · 2020-04-13T18:44:17Z

frontend/packages/knative-plugin/src/components/add/event-sources/YAMLEditorSection.tsx

+
+const YAMLEditorSection: React.FC<YAMLEditorSectionProps> = ({ eventSourceType }) => {
+  const { setFieldValue, setFieldTouched } = useFormikContext<FormikValues>();
+  const [namespaceField] = useField('project.name');


Add another prop for namespace instead of using useField.

rohitkrai03 · 2020-04-13T18:44:38Z

frontend/packages/knative-plugin/src/components/add/event-sources/YAMLEditorSection.tsx

+  React.useEffect(() => {
+    setFieldValue('yamlData', safeDump(getEventSourceYaml(namespaceField.value, eventSourceType)));
+    setFieldTouched('yamlData', true);
+    // eslint-disable-next-line react-hooks/exhaustive-deps


Why have you disabled the linting rule here?

while using useField, the dependency was not allowing the component to load.
removed disable linting rule after updating code to take namespace as a prop

rohitkrai03 · 2020-04-13T18:47:04Z

frontend/packages/knative-plugin/src/utils/create-eventsources-utils.ts

+  return eventSourceResource;
+};
+
+const getResourceFromForm = (formData: any): K8sResourceKind => {


Suggested change

const getResourceFromForm = (formData: any): K8sResourceKind => {

const getResourceFromFormData = (formData: any): K8sResourceKind => {

rohitkrai03 · 2020-04-13T18:54:15Z

frontend/packages/knative-plugin/src/utils/create-eventsources-utils.ts

+    case Object.values(KnownEventSources).find(
+      (knownEventSource) => formData.type === knownEventSource,
+    ):
+      return getResourceFromForm(formData);


Suggested change

case Object.values(KnownEventSources).find(

(knownEventSource) => formData.type === knownEventSource,

):

return getResourceFromForm(formData);

case KnownEventSources.CronJobSource:

case KnownEventSources.ContainerSource:

case KnownEventSources.ApiServerSource:

case KnownEventSources.SinkBinding:

return getResourceFromForm(formData);

rohitkrai03 · 2020-04-14T09:18:14Z

frontend/packages/knative-plugin/src/components/add/EventSourceForm.tsx

    <FormFooter
      handleReset={handleReset}
      errorMessage={status && status.submitError}
      isSubmitting={isSubmitting}
      submitLabel="Create"
-      sticky


Why have you removed sticky here? I thought all of our forms need to have sticky form footer,

@nemesis09 yes we need that

it slipped while resolving merge conflicts. added sticky

rohitkrai03 · 2020-04-14T09:23:59Z

frontend/packages/knative-plugin/src/utils/create-eventsources-utils.ts

+import ApiServerSection from '../components/add/event-sources/ApiServerSection';
+import KafkaSourceSection from '../components/add/event-sources/KafkaSourceSection';
+// eslint-disable-next-line import/no-cycle
+import YAMLEditorSection from '../components/add/event-sources/YAMLEditorSection';


Why have you disabled this eslint rule here? You cannot just disable any eslint rule that you encounter, they're there for a reason. This creates a cyclic dependency. You cannot have the eventSourceSectionLoader in the utils. Move it out to a separate component.

moved the util out to a separate file

nemesis09 · 2020-04-16T18:46:42Z

/retest

serenamarie125

@nemesis09 should the Create button be enabled by default? I thought that the provided YAML should be enough for the user to create an event source without having to provide any additional information. If that's the case Create should be enabled by default. FYI @invincibleJai

rohitkrai03 · 2020-04-16T20:10:57Z

frontend/packages/knative-plugin/src/components/add/event-sources/EventSourceSection.tsx

+
+const EventSourceSection: React.FC<EventSourceSectionProps> = ({ projects, namespace }) => {
+  const { values } = useFormikContext<FormikValues>();
+  let EventSource: React.ReactElement;


Add this line here instead of doing the check in default case - if (!values.type) return null. We don't want to render anything as the section unless and until a type is selected.

rohitkrai03 · 2020-04-16T20:11:41Z

frontend/packages/knative-plugin/src/components/add/event-sources/EventSourceSection.tsx

+      EventSource = <ContainerSourceSection />;
+      break;
+    default:
+      EventSource = values.type && !isKnownEventSource(values.type) ? <YAMLEditorSection /> : null;


Once you have the above check you can simply say -

Suggested change

EventSource = values.type && !isKnownEventSource(values.type) ? <YAMLEditorSection /> : null;

EventSource = <YAMLEditorSection />;

rohitkrai03 · 2020-04-16T20:12:45Z

frontend/packages/knative-plugin/src/components/add/eventSource-validation-utils.ts

+    });
+  }
+  return yup.object().shape({
+    yamldata: yup.string(),


Suggested change

yamldata: yup.string(),

yamlData: yup.string(),

nemesis09 · 2020-04-16T20:25:01Z

@nemesis09 should the Create button be enabled by default? I thought that the provided YAML should be enough for the user to create an event source without having to provide any additional information. If that's the case Create should be enabled by default. FYI @invincibleJai

Currently the yaml editor loads with a prefilled common structure for the event source with an empty spec and the user has to fill in the spec according to the event source requirements.

invincibleJai · 2020-04-16T20:25:52Z

@nemesis09 should the Create button be enabled by default? I thought that the provided YAML should be enough for the user to create an event source without having to provide any additional information. If that's the case Create should be enabled by default. FYI @invincibleJai

@serenamarie125 yes Create button should be enabled by default , there is a validation issue with formik which we are facing in some places will log an issue to track it @nemesis09 pls log an issue for it.

Provided yaml won't be complete as we don't have information from CRDs so providing with apiGroup/version/kind and sink in spec

rohitkrai03

/approve

nemesis09 · 2020-04-16T20:37:21Z

jira issue tracking the bug for validation
https://issues.redhat.com/browse/ODC-3591

invincibleJai · 2020-04-16T21:02:15Z

/restest

invincibleJai · 2020-04-16T21:44:32Z

/retest

nemesis09 · 2020-04-16T23:08:38Z

/test e2e-gcp-console

serenamarie125

Thanks for the answers @nemesis09 and @invincibleJai ! Approved by UX

rohitkrai03 · 2020-04-17T05:48:03Z

/lgtm

invincibleJai · 2020-04-17T06:13:54Z

Verified locally, works as expected

/lgtm

openshift-ci-robot · 2020-04-17T06:14:26Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: invincibleJai, nemesis09, rohitkrai03, serenamarie125

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~frontend/packages/dev-console/OWNERS~~ [invincibleJai,rohitkrai03]
~~frontend/packages/knative-plugin/OWNERS~~ [invincibleJai,rohitkrai03]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-bot · 2020-04-17T07:41:07Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-04-17T08:07:07Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot added the component/knative Related to knative-plugin label Apr 8, 2020

openshift-ci-robot requested review from andrewballantyne and debsmita1 April 8, 2020 04:42

openshift-ci-robot assigned rohitkrai03 Apr 8, 2020

rohitkrai03 suggested changes Apr 8, 2020

View reviewed changes

christianvogt reviewed Apr 8, 2020

View reviewed changes

nemesis09 force-pushed the camel-form branch 2 times, most recently from 232b781 to 8250960 Compare April 9, 2020 09:16

invincibleJai reviewed Apr 9, 2020

View reviewed changes

openshift-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 9, 2020

rohitkrai03 suggested changes Apr 9, 2020

View reviewed changes

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 9, 2020

nemesis09 force-pushed the camel-form branch 2 times, most recently from a70e0ae to 7851ca0 Compare April 13, 2020 01:22

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 13, 2020

nemesis09 force-pushed the camel-form branch 2 times, most recently from d55ede8 to 5ec1670 Compare April 13, 2020 02:14

rohitkrai03 suggested changes Apr 13, 2020

View reviewed changes

nemesis09 force-pushed the camel-form branch from 5ec1670 to 0afceec Compare April 13, 2020 17:35

nemesis09 force-pushed the camel-form branch from 0afceec to 8a3f6f0 Compare April 13, 2020 17:54

rohitkrai03 suggested changes Apr 13, 2020

View reviewed changes

nemesis09 force-pushed the camel-form branch from 8a3f6f0 to f6f7950 Compare April 13, 2020 21:10

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 13, 2020

nemesis09 force-pushed the camel-form branch from f6f7950 to 4d82c52 Compare April 13, 2020 21:23

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 13, 2020

rohitkrai03 suggested changes Apr 14, 2020

View reviewed changes

nemesis09 force-pushed the camel-form branch from d244313 to 85f2af8 Compare April 16, 2020 17:27

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 16, 2020

nemesis09 force-pushed the camel-form branch from 85f2af8 to a78d1da Compare April 16, 2020 17:46

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 16, 2020

nemesis09 force-pushed the camel-form branch 2 times, most recently from 8f3b5f8 to 37414af Compare April 16, 2020 19:56

serenamarie125 reviewed Apr 16, 2020

View reviewed changes

rohitkrai03 suggested changes Apr 16, 2020

View reviewed changes

nemesis09 force-pushed the camel-form branch from 37414af to a39f7fa Compare April 16, 2020 20:31

rohitkrai03 approved these changes Apr 16, 2020

View reviewed changes

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 16, 2020

add support for creating camel source

b9e072d

nemesis09 force-pushed the camel-form branch from a39f7fa to b9e072d Compare April 16, 2020 21:25

serenamarie125 approved these changes Apr 16, 2020

View reviewed changes

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 17, 2020

openshift-ci-robot assigned invincibleJai Apr 17, 2020

openshift-merge-robot merged commit 5dc9aa8 into openshift:master Apr 17, 2020

spadgett added this to the v4.5 milestone Apr 20, 2020

	<YAMLEditorField name={'yamlData'} />
	<YAMLEditorField name="yamlData" />

		} from '../../utils/create-eventsources-utils';
		import * as _ from 'lodash';

		setFieldValue('editorType', EditorType.Form);
		setFieldTouched('editorType', true);

-  project: yup.object().when('editorType', {
-    is: EditorType.Form,
-    then: projectNameValidationSchema,
-  }),
-  application: yup.object().when('editorType', {
-    is: EditorType.Form,
-    then: applicationNameValidationSchema,
-  }),
-  name: yup.string().when('editorType', {
+export const eventSourceValidationSchema = yup.object().when('type', {
+  is: (type) => Object.keys(EventSources).includes(type),
+  then: yup.object().shape({
+    project: projectNameValidationSchema,
+    application: applicationNameValidationSchema,
+    name: nameValidationSchema,
+    sink: sinkServiceSchema,
+    data: sourceDataSpecSchema,
+  }),
+  otherwise: yup.object().shape({
+    data: sourceDataSpecSchema,
+  }),
+});

		const eventSourceItems = Object.keys(eventSourceList).length;
		const handleItemChange = React.useCallback(

	const getResourceFromForm = (formData: any): K8sResourceKind => {
	const getResourceFromFormData = (formData: any): K8sResourceKind => {

	EventSource = values.type && !isKnownEventSource(values.type) ? <YAMLEditorSection /> : null;
	EventSource = <YAMLEditorSection />;

add support for creating camel source #4960

add support for creating camel source #4960

Conversation

nemesis09 commented Apr 8, 2020 • edited

invincibleJai commented Apr 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

invincibleJai Apr 9, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

invincibleJai commented Apr 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

invincibleJai commented Apr 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nemesis09 commented Apr 13, 2020

nemesis09 commented Apr 8, 2020 •

edited

invincibleJai Apr 9, 2020 •

edited